Repository navigation
fix(ci): pin vllm<0.19.1 to restore embedding test correctness - #1251
Conversation
vLLM 0.19.1 bundled a transformers v5 upgrade (transformers 4.57.6 → 5.5.4) that broke e5-mistral-7b-instruct embedding quality. e2e-1gpu-embeddings has failed on main since ~2026-04-18 16:00 UTC: TestEmbeddingCorrectness::test_semantic_similarity reports self-similarity 0.3342 (expected ~1.0), and test_relevance_scores reports scores beyond tolerance on both grpc and http paths. Bisect (main-branch PR Test runs of e2e-1gpu-embeddings (vllm)): - 24591985132 / 82a3fb1 vllm==0.19.0 transformers==4.57.6 -> SUCCESS (2026-04-18 04:35) - 24608587304 / dcede34 vllm==0.19.1 transformers==5.5.4 -> FAILURE (2026-04-18 16:21) Only smg commit between them is dcede34 'feat(protocols): add skills core types' — pure protocol types, unrelated to embeddings. The e2e install uses 'uv pip install vllm' with no pin, so CI resolves to whatever is current on PyPI at run time. The vLLM 0.19.1 release notes explicitly include 'Transformers v5 library upgrade'. Release notes do not document any embedding pooling or tokenizer change, but the vllm 0.19.1 + transformers 5.x pair clearly regresses e5-mistral embedding quality in practice; the sglang embeddings job passes on the same commit because it uses its own embedding pipeline. Full job URL: https://github.com/lightseekorg/smg/actions/runs/24644816475/job/72068881582 Fix: - scripts/ci_install_vllm.sh: change 'uv pip install vllm' to 'uv pip install "vllm<0.19.1"'. One-line change. Reverts CI to the last-known-good vllm version (0.19.0), which in turn transitively pins transformers back to 4.57.x. This is a temporary pin. Upstream fix path (in priority order): 1. Upstream vllm 0.19.2+ that re-verifies embedding-model correctness post-transformers-v5 — upgrade the pin then. 2. If vllm never re-verifies, pin 'transformers<5' alongside vllm separately. Either way, this pin should be revisited the next time vllm publishes a new release; leaving this open as follow-up rather than removing the pin on faith. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughConstrain vLLM install to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request pins the vLLM package to a version below 0.19.1 in the CI installation script to resolve a regression in embedding quality caused by a dependency upgrade. The review feedback suggests pinning to the exact version 0.19.0 instead of using a version range to ensure better stability and reproducibility in the CI environment.
| # 72068881582 for the failure signature. | ||
| echo "Installing vLLM..." | ||
| uv pip install vllm | ||
| uv pip install "vllm<0.19.1" |
There was a problem hiding this comment.
To ensure CI stability and consistency with the project's metadata, consider pinning to the exact last-known-good version (vllm==0.19.0). This matches the version cited in the comments and the lower bound defined in grpc_servicer/pyproject.toml. Using an exact pin is generally preferred in CI to ensure reproducibility, especially when a specific version is known to cause regressions. Additionally, since this regression affects core functionality (embedding correctness), this constraint should ideally be reflected in the package metadata (grpc_servicer/pyproject.toml) as well to protect users from installing the broken combination. This approach also adheres to the requirement of using uv pip install for Python package installations in CI scripts for consistency and performance.
| uv pip install "vllm<0.19.1" | |
| uv pip install "vllm==0.19.0" |
References
- When installing Python packages in CI scripts, use uv pip install for consistency and performance if uv is already in use for other installations.
- Ensure that all external resource versions specified in scripts are accurate and exist, as incorrect versions can lead to build failures.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ci_install_vllm.sh`:
- Line 30: Replace the floating vllm constraint in the pip install line that
currently reads 'uv pip install "vllm<0.19.1"' with an exact pin to the
last-known-good release (vllm==0.19.0) so CI uses deterministic dependencies;
update the install command string accordingly to use "vllm==0.19.0".
🪄 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: f28441f4-febc-4fca-b0c0-ea91fc769859
📒 Files selected for processing (1)
scripts/ci_install_vllm.sh
Addendum to the prior commit on this PR. The original fix pinned only vllm<0.19.1, which relies on vllm 0.19.0's wheel metadata (transformers<5,>=4.56.0) to force transformers back to 4.x. That works today, but it delegates the transformers ceiling to upstream metadata we don't control. If vllm ever publishes a 0.19.0 hotfix that relaxes its transformers constraint, the pin silently regresses. Adding an explicit transformers<5 ceiling makes the install deterministic even if vllm 0.19.0's metadata changes. Restores the last-known-good combo (vllm==0.19.0, transformers==4.57.6) unambiguously. Still does not isolate whether the underlying regression is in vllm 0.19.1 or transformers 5.x — that's a follow-up experiment; see PR description. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Reverts the second commit on this PR. We should not second-guess vllm's declared transformers range. vllm 0.19.0's wheel already hard-caps transformers<5,>=4.56.0, so pinning vllm<0.19.1 is sufficient to get the last-known-good combo. When we ship vllm in production, users don't pin transformers themselves either — keeping CI aligned with the install shape real users will get. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
The detect-changes path filter enumerates which scripts should trigger the GPU e2e matrix. It listed ci_install_sglang.sh but omitted ci_install_vllm.sh — so PRs that only touch the vllm install script (including this one) got every GPU e2e job SKIPPED, leaving no way to verify the vllm-side fix the PR is landing. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/ci_install_vllm.sh (1)
30-36:⚠️ Potential issue | 🟠 MajorPinning intent and install constraint are misaligned on Line 30 and Line 36.
The comment says we rely on vLLM 0.19.0 metadata, but
vllm<0.19.1is still a floating selector. For deterministic CI recovery, pin exactly to the known-good version.🔧 Proposed fix
-# We rely on vllm 0.19.0's own wheel metadata (transformers<5,>=4.56.0) to +# We rely on vllm 0.19.0's own wheel metadata (transformers<5,>=4.56.0) to # force transformers back to 4.x. Not pinning transformers separately — if # we ship vllm we don't want to second-guess the library's own dep range; # drop this pin whenever vllm publishes a release that re-verifies embedding # correctness post-transformers-v5. echo "Installing vLLM..." -uv pip install "vllm<0.19.1" +uv pip install "vllm==0.19.0"Use this read-only check to confirm resolver surface and current script intent:
#!/bin/bash set -euo pipefail # Verify local script context cat -n scripts/ci_install_vllm.sh | sed -n '22,37p' # Inspect published versions to see what "<0.19.1" could resolve to python - <<'PY' import json, urllib.request from packaging.version import Version data = json.load(urllib.request.urlopen("https://pypi.org/pypi/vllm/json")) versions = sorted(Version(v) for v in data.get("releases", {}).keys()) lt_0191 = [v for v in versions if v < Version("0.19.1")] range_0190_0191 = [v for v in versions if Version("0.19.0") <= v < Version("0.19.1")] print("Latest five versions <0.19.1:", [str(v) for v in lt_0191[-5:]]) print("Candidates in [0.19.0, 0.19.1):", [str(v) for v in range_0190_0191]) PYExpected result after pinning: install command is
vllm==0.19.0, and CI resolution no longer depends on future sub-0.19.1 release changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ci_install_vllm.sh` around lines 30 - 36, The comment and install constraint disagree: update the vLLM install to pin the tested version exactly rather than a floating range; replace the install invocation that currently uses the string "vllm<0.19.1" with a fixed "vllm==0.19.0" and adjust the preceding comment text so it explicitly states we pin to 0.19.0 (refer to the echo "Installing vLLM..." line and the pip install command in the script).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/ci_install_vllm.sh`:
- Around line 30-36: The comment and install constraint disagree: update the
vLLM install to pin the tested version exactly rather than a floating range;
replace the install invocation that currently uses the string "vllm<0.19.1" with
a fixed "vllm==0.19.0" and adjust the preceding comment text so it explicitly
states we pin to 0.19.0 (refer to the echo "Installing vLLM..." line and the pip
install command in the script).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 968d5493-9484-49a8-a811-fec9b78e1f7e
📒 Files selected for processing (1)
scripts/ci_install_vllm.sh
2 participants
Description
Problem
`e2e-1gpu-embeddings (vllm)` has failed on every main-branch run since ~2026-04-18 16:00 UTC. The failure is not a test infrastructure issue —
TestEmbeddingCorrectness::test_semantic_similarityreports self-similarity 0.3342 instead of ~1.0 on `intfloat/e5-mistral-7b-instruct`. That is broken pooling / tokenizer, not numerical drift.Latest failing run: https://github.com/lightseekorg/smg/actions/runs/24644816475/job/72068881582
Root cause (bisected against main)
82a3fb1a, 2026-04-18 04:35)dcede344, 2026-04-18 16:21)vllmtransformerstorchOnly SMG commit in that 12-hour window was
dcede344 feat(protocols): add skills core types— pure protocol types, unrelated to embeddings.scripts/ci_install_vllm.shrunsuv pip install vllmwith no version pin, so CI resolves to whatever is current on PyPI. The vLLM 0.19.1 release notes explicitly include a "Transformers v5 library upgrade". Release notes don't document any embedding pooling change, but thevllm 0.19.1 + transformers 5.xpair clearly regressese5-mistralembedding quality in practice. `e2e-1gpu-embeddings (sglang)` passed on the same commit because sglang has its own embedding pipeline.Solution
Pin `vllm<0.19.1` in `scripts/ci_install_vllm.sh`. One-line change; reverts CI to the last-known-good combo (
vllm==0.19.0/transformers==4.57.x).Changes
Test Plan
Follow-up (not blocking this PR)
This is a temporary pin. Revisit next time vllm publishes a release:
Either way, don't relax this pin without re-running the embedding correctness suite against the new combo.
Checklist
Summary by CodeRabbit