Repository navigation
[CI] ci: Run the PR review guideline's checks on dedicated MI355X nodes - #107
Open
michaelzhang-ai wants to merge 2 commits into
Open
michaelzhang-ai wants to merge 2 commits into
michaelzhang-ai wants to merge 2 commits into
Conversation
… a job gone, keep evals within the covered width
michaelzhang-ai
marked this pull request as ready for review
November 5, 2026 21:04
michaelzhang-ai
requested review from
HaiShaw,
alexnails and
kevin-mii
as code owners
November 5, 2026 21:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A maintainer-gated PR test that runs the checks of the PR review guideline on two dedicated MI355X nodes: for each model a PR touches, its unit, kernel and serving tests, the kernel-only timing scripts, sgl-eval accuracy and InferenceX fixed-length speed. Accuracy and speed are compared against the base branch, run on the same node with the same server recipe. The workflow posts the result as an
AMD PR testcommit status and keeps one PR comment updated with a checklist in the guideline's order (rules, scope, accuracy, kernel microbenchmark, E2E performance, tests, doc).Why
The guideline asks reviewers to approve only when these checks pass, plus a ≥5% gain for performance work. Today authors run the checks by hand and paste the numbers. A run takes hours and this repo should change rarely, so the test starts only when a maintainer starts it.
How it works
run-cilabel, or runs the workflow with a PR number. The run removes the label and pins the PR head SHA it saw, so any later push needs a maintainer again.scripts/ci/scope.py): a model whose own package or tests changed runs the full tier. A change tocommon/,__init__.pyorpyproject.tomlruns every other model's smoke tier (tests, plus speed at the covered widths only), andcommon(the unit tests and the guard/dispatch kernel tests).scripts/ci/models.py,run_model.py): the base branch is installed first, then the PR, over the image's sgl-amd-mono. Both sides use the PR'stest/e2erecipe with--enable-amd-monokernel. A model that is new in the PR is compared against SGLang's own path (monokernels off). The base accuracy result is cached per image and base commit.report.py): the run fails on any failed test, a server that logs no monokernel routing, accuracy below base by more than the per-benchmark tolerance, or a median-TPOT loss of more than 2%.[Perf]and[Feat]PRs also need a ≥5% median-TPOT gain at one of the model's covered concurrencies. The run also fails on edits to existing.github/CODEOWNERSlines; an added owner line for a new model is only a warning. A missing model README.md, or a check that can't run on these nodes, is a warning.pull_request_target, and the scripts that run outside the container come from the base branch. PR code runs only inside the SGLang ROCm container. The cluster job fetches both commits itself.Notes for the reviewer
scripts/ci/runner.sh, labelsgl-amd-mono-mi355x), create therun-cilabel, and, if wanted, makeAMD PR testa required status.Test plan
scope.pyagainst the file lists of [Perf] [MI355X] deepseek_v41: DSpark step glue in fewer launches, Markov draft sampling, faster K1 and K2 #95, [Chore] maintainer: updates #96, [Fix] [MI355X] glm53_flash: Stop the sparse-attention fault on rows past 262,144 tokens #97, [Feat] common: Check each compiled kernel's residency and scratch where a runner builds it #101, [Feat] [MI355X] kimi_k3/kimi_k31: Kimi-K3's gfx950 launches, and Kimi-K3.1 steps of up to 8 rows on them #103 and [Refactor] [MI355X] kimi_k3/kimi_k31: One serving form, no copied helpers, every wide width a bucket #105; the models and rule results match what each PR touched ([Chore] maintainer: updates #96 and [Feat] [MI355X] kimi_k3/kimi_k31: Kimi-K3's gfx950 launches, and Kimi-K3.1 steps of up to 8 rows on them #103 trip the CODEOWNERS rule).kimi_k31job passed its kernel test (test_kimi_k31_k1_ring.py) and ranlayer_bench.py;v0.5.23nightly 20261104: 579 passed, 11 failed (the kimi_k3/k31 forward-drift and glm53_flash spec tests). These fail on main too, so they are an SGLang-version issue, not this change.