test(embeddings): enable sglang embedding correctness on h100 by CatherineSue · Pull Request #1326 · smg-project/smg

CatherineSue · 2026-04-22T17:42:00Z

Description

Problem

Two things prevent the sglang embedding correctness check from actually running:

e2e-1gpu-embeddings is pinned to the 1-gpu runner pool, while e2e-1gpu-completions already runs on 1-gpu-h100.
TestEmbeddingCorrectness carries a skip_for_runtime("sglang", reason="sglang embedding output diverges from HF reference") mark, so the sglang side of the comparison never executes.

Solution

Move the embeddings job onto H100 nodes and drop the sglang skip so the gateway-vs-HF reference comparison runs end-to-end on sglang.

Changes

.github/workflows/pr-test-rust.yml: e2e-1gpu-embeddings runner 1-gpu → 1-gpu-h100.
e2e_test/embeddings/test_correctness.py: remove @pytest.mark.skip_for_runtime("sglang", ...) on TestEmbeddingCorrectness.

Test Plan

CI on this PR runs e2e-1gpu-embeddings (sglang) on the 1-gpu-h100 pool.
TestEmbeddingCorrectness executes for both sglang and vllm engines (no longer reported as skipped for sglang).
If the sglang correctness assertion fails, the failure surfaces here rather than being silently skipped.

Checklist

cargo +nightly fmt passes (N/A — no Rust changes)
cargo clippy --all-targets --all-features -- -D warnings passes (N/A — no Rust changes)
(Optional) Documentation updated
(Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

Tests
- Enabled embedding correctness validation tests that were previously skipped, improving test coverage and ensuring consistent embedding output quality across supported inference engines.
Chores
- Updated CI infrastructure and test runner specifications to use optimized GPU execution environments.

- Switch e2e-1gpu-embeddings runner from 1-gpu to 1-gpu-h100, matching e2e-1gpu-completions. - Drop skip_for_runtime("sglang") on TestEmbeddingCorrectness so the sglang vs HF reference comparison actually runs. Signed-off-by: Chang Su <chang.s.su@oracle.com>

gemini-code-assist · 2026-04-22T17:42:04Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

coderabbitai · 2026-04-22T17:42:13Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6230cb10-6408-48b3-b624-d57f7bb0306f

📥 Commits

Reviewing files that changed from the base of the PR and between a92cfb3 and e64065b.

📒 Files selected for processing (2)

.github/workflows/pr-test-rust.yml
e2e_test/embeddings/test_correctness.py

💤 Files with no reviewable changes (1)

e2e_test/embeddings/test_correctness.py

📝 Walkthrough

Walkthrough

This PR updates the embeddings test CI workflow to use H100-based GPU runners and enables sglang embedding correctness tests by removing a previously applied skip decorator, indicating that the embedding output divergence issue has been resolved.

Changes

Cohort / File(s)	Summary
CI Workflow Configuration `.github/workflows/pr-test-rust.yml`	Updated the `e2e-1gpu-embeddings` job runner from `1-gpu` to `1-gpu-h100` to use H100 GPU hardware.
Test Markers `e2e_test/embeddings/test_correctness.py`	Removed the skip decorator for sglang runtime in embedding correctness tests, enabling previously skipped test execution.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

fix(ci): prevent runner shutdown on concurrent main pushes and restore path filters #678: Modifies the same e2e-1gpu-embeddings CI job with gating/needs/if logic adjustments.
fix(ci): Use H100 runner for Specific Tests #357: Updates runner selection to H100 GPU variants across GitHub Actions workflows.

Suggested labels

ci, tests

Suggested reviewers

key4ng
slin1237
XinyueZhang369

Poem

🐰 H100 runners shine so bright,
Skip marks vanish in the light,
Embeddings dance without delay,
Tests bloom with correctness today! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: enabling sglang embedding correctness testing on h100 hardware by both upgrading the runner and removing a skip decorator.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch test/embeddings-h100-enable-sglang-correctness

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

claude

Reviewed: 2 files changed. No issues found.

Runner upgrade from 1-gpu to 1-gpu-h100 aligns with all other 1-GPU e2e jobs in the workflow.
Removing the skip_for_runtime("sglang") marker enables the embedding correctness test on H100, where the output divergence from HF reference is presumably resolved.

Clean change, LGTM.

PR #1326 removed `@pytest.mark.skip_for_runtime("sglang", reason="sglang embedding output diverges from HF reference")` on the hypothesis that H100 hardware would close the sglang vs HuggingFace reference gap for `intfloat/e5-mistral-7b-instruct`. Empirically that is not the case: PR #1329 CI (sglang 0.5.10 and 0.5.10.post1 on H100 arc-runner-1-gpu-h100-gwjtg-*): FAILED test_semantic_similarity[grpc] AssertionError: Set 1, text 1: similarity 0.3342 not close to 1.0 FAILED test_relevance_scores[grpc,http] AssertionError: Scores differ beyond tolerance 0.3342 vs 1.0 is structural (pooling / normalization / instruction prefix), not hardware — H100 alone does not close the gap. Keep the H100 runner change from #1326 (embeddings lane still runs on 1-gpu-h100) so that when the underlying sglang divergence is fixed, re-enabling this test is a one-line revert of this commit. Restore only the skip decorator to unblock the sglang-embeddings CI lane in the meantime. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>

sentence-transformers >= 3.x top-level imports torchcodec via modality_types, which dlopens libtorchcodec_core{4..8}.so linked against FFmpeg shared libs (libavformat/libavcodec/libavutil/ libswresample/libswscale). The runner image does not ship these. PR #1326 removed the `skip_for_runtime("sglang", ...)` decorator on test_correctness.py, so from that merge onward every sglang embeddings run imports torchcodec at fixture setup and errors with: RuntimeError: Could not load libtorchcodec. Likely causes: 1. FFmpeg is not properly installed ... Previously the test was skipped on sglang, which hid the missing libav* on the runner image. Gated strictly on the embeddings matrix via the `sentence-transformers` marker in extra_deps (pr-test-rust.yml:511). Other CI lanes see no apt traffic. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>

CatherineSue requested review from XinyueZhang369, key4ng and slin1237 as code owners April 22, 2026 17:42

github-actions Bot added ci CI/CD configuration changes tests Test changes labels Apr 22, 2026

claude Bot approved these changes Apr 22, 2026

View reviewed changes

coderabbitai Bot approved these changes Apr 22, 2026

View reviewed changes

key4ng approved these changes Apr 22, 2026

View reviewed changes

slin1237 merged commit 614ccc0 into main Apr 22, 2026
39 of 42 checks passed

slin1237 deleted the test/embeddings-h100-enable-sglang-correctness branch April 22, 2026 18:31

This was referenced Apr 22, 2026

fix(ci): install ffmpeg OS libs for embeddings extras #1329

Closed

test(embeddings): restore skip_for_runtime("sglang") on correctness test #1330

Merged

Navigation Menu

Repository navigation

test(embeddings): enable sglang embedding correctness on h100 - #1326

test(embeddings): enable sglang embedding correctness on h100#1326
slin1237 merged 1 commit into
mainfrom
test/embeddings-h100-enable-sglang-correctness

CatherineSue commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

gemini-code-assist Bot commented Apr 22, 2026

Uh oh!

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

CatherineSue commented Apr 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Changes

Test Plan

Summary by CodeRabbit

Uh oh!

gemini-code-assist Bot commented Apr 22, 2026

Uh oh!

coderabbitai Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CatherineSue commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading