ci(benchmarks): add Devstral 2 to H200 nightly benchmarks#1069
ci(benchmarks): add Devstral 2 to H200 nightly benchmarks#1069smfirmin wants to merge 2 commits intolightseekorg:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 48 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds support for Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 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)
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 adds the Devstral-2-123B-Instruct-2512 model to the nightly performance benchmarks and infrastructure configurations. It also introduces a mechanism to skip redundant multi-worker test variants for specific models. The reviewer suggests simplifying the test generation logic by checking if _multi_workers > 1 instead of using a hardcoded exclusion set, which would more generically handle models that only require a single worker.
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)
e2e_test/benchmarks/test_nightly_perf.py (1)
176-188: 🧹 Nitpick | 🔵 TrivialClean up
_variantsfrom module namespace for consistency.Line 188 cleans loop temporaries, but
_variantsis now also a module-level temporary and should be deleted too.Suggested small cleanup
-del _model_id, _name, _multi_workers, _backends, _extra, _suffix, _count, _cls_name, _cls +del _model_id, _name, _multi_workers, _backends, _extra, _variants, _suffix, _count, _cls_name, _cls🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e_test/benchmarks/test_nightly_perf.py` around lines 176 - 188, The module-level temporary _variants is left behind after the loop and should be removed with the other temporaries; add _variants to the cleanup deletion so the trailing line becomes del _model_id, _name, _multi_workers, _backends, _extra, _suffix, _count, _cls_name, _cls, _variants to avoid leaking the variable from the loop that builds test classes in the block using _NIGHTLY_MODELS and _make_test_class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-benchmark.yml:
- Line 353: The YAML flow-mapping on the line containing the mapping with id:
mistralai/Devstral-2-123B-Instruct-2512, slug:
mistralai-Devstral-2-123B-Instruct-2512, and test_class:
TestNightlyDevstral2Single violates the `braces` rule due to spaces immediately
inside `{ }`; remove the space after `{` and before `}` so the mapping becomes
brace-adjacent (e.g. `{id: ..., slug: ..., test_class: ...}`) to satisfy
yamlint.
---
Outside diff comments:
In `@e2e_test/benchmarks/test_nightly_perf.py`:
- Around line 176-188: The module-level temporary _variants is left behind after
the loop and should be removed with the other temporaries; add _variants to the
cleanup deletion so the trailing line becomes del _model_id, _name,
_multi_workers, _backends, _extra, _suffix, _count, _cls_name, _cls, _variants
to avoid leaking the variable from the loop that builds test classes in the
block using _NIGHTLY_MODELS and _make_test_class.
🪄 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: 419636fa-5ef2-4f14-a65a-ea79ac9fe15d
📒 Files selected for processing (3)
.github/workflows/nightly-benchmark.ymle2e_test/benchmarks/test_nightly_perf.pye2e_test/infra/model_specs.py
|
Hi @smfirmin, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
efd72de to
34a818c
Compare
Description
Summary
Add
mistralai/Devstral-2-123B-Instruct-2512to the nightly benchmark coverage on H200 for bothsglangandvllm.Changes
MODEL_SPECSwith nightly benchmark metadata4single-worker-h200workflow matrixminimax-m2benchmark entryNotes
sglangandvllmSummary by CodeRabbit
Release Notes
New Features
mistralai/Devstral-2-123B-Instruct-2512model with chat, streaming, function calling, and reasoning capabilities.Tests