[IN REVIEW] refactor(multimodal): use standalone llm-multimodal crate - #1911
[IN REVIEW] refactor(multimodal): use standalone llm-multimodal crate#1911BugenZhao wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe PR removes the in-repository multimodal crate, switches ChangesMultimodal dependency transition
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MultimodalPlan
participant MultimodalTokenizer
participant TokenizerTrait
participant ModelMetadata
MultimodalPlan->>MultimodalTokenizer: wrap tokenizer
MultimodalTokenizer->>TokenizerTrait: forward token lookup and text encoding
MultimodalTokenizer->>ModelMetadata: provide external tokenizer implementation
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request removes the crates/multimodal crate from the workspace, updates related configuration files like CODEOWNERS and labeler.yml, and updates documentation references. Additionally, the llm-multimodal dependency is now sourced from a remote git repository. I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-test-rust.yml (1)
302-303: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale fixture-generation reference.
.gitignore:130still instructs developers to run the removedgenerate_vision_golden.pyscript. Delete or update that entry in this PR so repository guidance matches the removal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr-test-rust.yml around lines 302 - 303, Update the stale `.gitignore` guidance entry for `generate_vision_golden.py` to remove it or replace it with the current fixture-generation workflow, ensuring repository instructions no longer reference the removed script.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-test-rust.yml:
- Line 275: Update the workflow step named “Verify default multimodal dependency
build does not require OpenCV” to stop invoking the non-workspace
`llm-multimodal` package via `cargo check -p`. Check an in-workspace consumer of
that git dependency or use an appropriate standalone manifest while preserving
the OpenCV-free default-build verification.
---
Outside diff comments:
In @.github/workflows/pr-test-rust.yml:
- Around line 302-303: Update the stale `.gitignore` guidance entry for
`generate_vision_golden.py` to remove it or replace it with the current
fixture-generation workflow, ensuring repository instructions no longer
reference the removed script.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cad9ddb4-6887-4103-8e5d-fd587c06d198
⛔ Files ignored due to path filters (10)
crates/multimodal/tests/fixtures/images/grayscale.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/large.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/odd_dims.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/small.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/square.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/tall.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/tiny.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/very_tall.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/very_wide.jpgis excluded by!**/*.jpgcrates/multimodal/tests/fixtures/images/wide.jpgis excluded by!**/*.jpg
📒 Files selected for processing (69)
.github/CODEOWNERS.github/labeler.yml.github/workflows/nightly-benchmark.yml.github/workflows/pr-test-rust.yml.github/workflows/release-crates.yml.pre-commit-config.yamlCargo.tomlcrates/grpc_client/proto/tokenspeed_scheduler.protocrates/multimodal/Cargo.tomlcrates/multimodal/benches/image_preprocess.rscrates/multimodal/build.rscrates/multimodal/scripts/generate_qwen_preprocess_fingerprints.pycrates/multimodal/scripts/generate_vision_golden.pycrates/multimodal/src/audio/decode.rscrates/multimodal/src/audio/mod.rscrates/multimodal/src/audio/processor.rscrates/multimodal/src/audio/processors/mod.rscrates/multimodal/src/audio/processors/qwen3_audio.rscrates/multimodal/src/audio/transforms.rscrates/multimodal/src/encoder_inputs.rscrates/multimodal/src/error.rscrates/multimodal/src/hasher.rscrates/multimodal/src/hub.rscrates/multimodal/src/jpeg_turbo.rscrates/multimodal/src/lib.rscrates/multimodal/src/media.rscrates/multimodal/src/opencv_buffer.rscrates/multimodal/src/opencv_buffer_capture.cppcrates/multimodal/src/registry/kimi_k25.rscrates/multimodal/src/registry/llama4.rscrates/multimodal/src/registry/llava.rscrates/multimodal/src/registry/mod.rscrates/multimodal/src/registry/phi3_v.rscrates/multimodal/src/registry/qwen3_asr.rscrates/multimodal/src/registry/qwen3_omni.rscrates/multimodal/src/registry/qwen3_vl.rscrates/multimodal/src/registry/qwen_vl.rscrates/multimodal/src/registry/traits.rscrates/multimodal/src/tracker.rscrates/multimodal/src/types.rscrates/multimodal/src/vision/execution.rscrates/multimodal/src/vision/mod.rscrates/multimodal/src/vision/preprocessor_config.rscrates/multimodal/src/vision/processor.rscrates/multimodal/src/vision/processors/kimi_k25.rscrates/multimodal/src/vision/processors/llama4_vision.rscrates/multimodal/src/vision/processors/llava.rscrates/multimodal/src/vision/processors/mod.rscrates/multimodal/src/vision/processors/phi3_vision.rscrates/multimodal/src/vision/processors/phi4_vision.rscrates/multimodal/src/vision/processors/pixtral.rscrates/multimodal/src/vision/processors/qwen2_vl.rscrates/multimodal/src/vision/processors/qwen3_omni_vision.rscrates/multimodal/src/vision/processors/qwen3_vl.rscrates/multimodal/src/vision/processors/qwen_vl_base.rscrates/multimodal/src/vision/scratch.rscrates/multimodal/src/vision/transforms.rscrates/multimodal/tests/decode_preprocess_bench.rscrates/multimodal/tests/fixtures/golden/qwen_preprocess_fingerprints.jsoncrates/multimodal/tests/multimodal_tracker_test.rscrates/multimodal/tests/preprocess_fingerprint.rscrates/multimodal/tests/qwen_preprocess_golden.rscrates/multimodal/tests/resize_fingerprint.rscrates/multimodal/tests/vision_golden_tests.rsmodel_gateway/Cargo.tomlmodel_gateway/src/routers/grpc/multimodal/mod.rsmodel_gateway/src/routers/grpc/multimodal/plan.rsmodel_gateway/src/routers/grpc/multimodal/process.rsscripts/check_release_versions.sh
💤 Files with no reviewable changes (60)
- crates/multimodal/tests/preprocess_fingerprint.rs
- crates/multimodal/tests/multimodal_tracker_test.rs
- crates/multimodal/Cargo.toml
- crates/multimodal/tests/fixtures/golden/qwen_preprocess_fingerprints.json
- crates/multimodal/tests/resize_fingerprint.rs
- crates/multimodal/build.rs
- crates/multimodal/src/media.rs
- crates/multimodal/src/error.rs
- crates/multimodal/benches/image_preprocess.rs
- .github/CODEOWNERS
- crates/multimodal/src/audio/processors/mod.rs
- crates/multimodal/src/vision/mod.rs
- crates/multimodal/src/vision/execution.rs
- crates/multimodal/src/hasher.rs
- crates/multimodal/src/opencv_buffer_capture.cpp
- crates/multimodal/tests/qwen_preprocess_golden.rs
- .pre-commit-config.yaml
- .github/workflows/release-crates.yml
- crates/multimodal/src/registry/traits.rs
- crates/multimodal/src/registry/llama4.rs
- crates/multimodal/src/vision/processors/mod.rs
- crates/multimodal/src/tracker.rs
- crates/multimodal/src/jpeg_turbo.rs
- crates/multimodal/src/registry/qwen3_omni.rs
- .github/labeler.yml
- crates/multimodal/src/opencv_buffer.rs
- crates/multimodal/src/registry/kimi_k25.rs
- crates/multimodal/src/registry/qwen_vl.rs
- crates/multimodal/src/registry/qwen3_asr.rs
- crates/multimodal/src/registry/phi3_v.rs
- crates/multimodal/src/audio/mod.rs
- crates/multimodal/src/registry/llava.rs
- crates/multimodal/src/encoder_inputs.rs
- crates/multimodal/src/hub.rs
- crates/multimodal/src/vision/processors/pixtral.rs
- crates/multimodal/tests/decode_preprocess_bench.rs
- crates/multimodal/src/vision/processors/phi4_vision.rs
- crates/multimodal/scripts/generate_vision_golden.py
- crates/multimodal/scripts/generate_qwen_preprocess_fingerprints.py
- crates/multimodal/src/audio/decode.rs
- crates/multimodal/src/registry/mod.rs
- crates/multimodal/src/lib.rs
- crates/multimodal/src/vision/processors/kimi_k25.rs
- crates/multimodal/src/vision/scratch.rs
- crates/multimodal/src/registry/qwen3_vl.rs
- crates/multimodal/src/vision/processors/llama4_vision.rs
- crates/multimodal/src/vision/processors/phi3_vision.rs
- scripts/check_release_versions.sh
- crates/multimodal/src/vision/processor.rs
- crates/multimodal/src/vision/preprocessor_config.rs
- crates/multimodal/src/types.rs
- crates/multimodal/src/vision/processors/llava.rs
- crates/multimodal/src/audio/processors/qwen3_audio.rs
- crates/multimodal/src/vision/processors/qwen3_omni_vision.rs
- crates/multimodal/src/vision/processors/qwen3_vl.rs
- crates/multimodal/src/vision/processors/qwen_vl_base.rs
- crates/multimodal/src/audio/processor.rs
- crates/multimodal/src/vision/transforms.rs
- crates/multimodal/src/audio/transforms.rs
- crates/multimodal/src/vision/processors/qwen2_vl.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ee8aafa58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Hi @BugenZhao, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
3ee8aaf to
9725c66
Compare
9725c66 to
8af1e38
Compare
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
8af1e38 to
5ab8477
Compare
|
Hi @BugenZhao, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
|
This pull request has been automatically marked as stale because it has not had any activity within 14 days. It will be automatically closed if no further activity occurs within 16 days. Leave a comment if you feel this pull request should remain open. Thank you! |
Description
Problem
llm-multimodalnow lives in a standalone repository, while SMG retained an in-tree copy. The two copies have already accumulated changes independently, creating duplicate ownership for preprocessing behavior, fixtures, CI, and releases.Solution
Use smg-project/llm-multimodal as the source of truth.
SMG now depends on the standalone crate at revision
7df38e53, while retainingversion = "1.7.1"so Cargo uses the pinned Git revision during development and the crates.io version when packaging SMG.Changes
TokenizerTraitto the standalone crate's narrowTokenizerinterface.rustls-tlsandhf-hub.opencv-videofeature forwarding from SMG.crates/multimodalimplementation and fixtures.Cargo.tomlchanges trigger the relevant integration tests.The diff of generated
Cargo.lockis clean:Test Plan
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
Summary by CodeRabbit
Refactor
llm-multimodalpackage (the in-repo multimodal crate was removed from the workspace).Chores
CI / Automation
Release
llm-multimodalfrom the tier2 publish matrix.