CollectiveX MVP v0.1 #1612
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
| name: CODEOWNER Sign-off Verify | |
| # Independently re-verifies a CODEOWNER reviewer sign-off. | |
| # | |
| # A reviewer marks a PR ready by posting the sign-off checklist that starts with | |
| # "As a PR reviewer and CODEOWNER" (see e.g. | |
| # https://github.com/SemiAnalysisAI/InferenceX/pull/1792#issuecomment-4781799476). | |
| # That sign-off can be posted three different ways, and all three trigger here: | |
| # - a conversation comment -> issue_comment | |
| # - an Approve/Comment review summary -> pull_request_review | |
| # - an inline code-review comment -> pull_request_review_comment | |
| # | |
| # When the sign-off lands on an open PR, this workflow asks Claude to | |
| # independently confirm the claims that actually gate a merge: | |
| # 0. The sign-off author is a CODEOWNER for the files the PR changes. | |
| # 1. PR validation has at least one fully-green run on a commit in the PR. | |
| # 2. Evals pass on that same green run. | |
| # 3. The single-node recipe(s) are linked in the sign-off AND the linked | |
| # recipe PR/commit contains all of the reproduction information that is in | |
| # this InferenceX PR. Single-node submissions only — disaggregated / | |
| # multi-node PRs (dynamo, ATOM/ATOMesh, sglang/vllm disagg) are N/A. | |
| # 4. An authorized maintainer has explicitly posted a `/reuse-sweep-run` | |
| # command on the PR (the reuse-merge path requires it). | |
| # 5. The sign-off uses the latest docs/PR_REVIEW_CHECKLIST.md template. | |
| # 6. vLLM/SGLang configs run upstream images (hub.docker.com/u/vllm, | |
| # hub.docker.com/u/lmsysorg) on established hardware, and vLLM/SGLang | |
| # submissions land before additional frameworks (TRT-LLM, ATOM, ...). | |
| # 7. No benchmark hacks that change the model architecture / cut FLOPs. | |
| # 8. Speculative-decoding configs benchmark through chat templates. | |
| # If any of those are not to standard, Claude posts a single comment that | |
| # @-mentions the reviewer who signed off and explains exactly what is wrong. | |
| # | |
| # The verdict is also published as a `codeowner-signoff-verify` commit status | |
| # on the PR head SHA (success only on "Verdict: PASS"). That status context is | |
| # a REQUIRED check in the "Protect main" ruleset, so a PR cannot be merged | |
| # (without org-admin bypass) until a sign-off has been posted AND independently | |
| # verified as passing on the current head commit. | |
| # | |
| # It can also be run manually via workflow_dispatch by passing the URL of the | |
| # sign-off comment/review to verify. | |
| on: | |
| issue_comment: | |
| types: [created, edited] | |
| pull_request_review: | |
| types: [submitted, edited] | |
| pull_request_review_comment: | |
| types: [created, edited] | |
| workflow_dispatch: | |
| inputs: | |
| comment_url: | |
| description: >- | |
| URL of the sign-off comment/review to verify, e.g. | |
| https://github.com/OWNER/REPO/pull/123#issuecomment-456 (also accepts | |
| #pullrequestreview-<id> and #discussion_r<id> review URLs). | |
| required: true | |
| type: string | |
| concurrency: | |
| group: codeowner-signoff-verify-${{ github.event.issue.number || github.event.pull_request.number || github.run_id }} | |
| cancel-in-progress: false | |
| jobs: | |
| # --------------------------------------------------------------------------- | |
| # Gate: only proceed for the sign-off checklist on an OPEN, non-draft PR. | |
| # Normalizes the three event shapes (issue_comment / pull_request_review / | |
| # pull_request_review_comment) into a single set of outputs, resolves an | |
| # immutable head SHA (TOCTOU-safe), and hands the verifier a ready-to-run | |
| # command for fetching the exact sign-off body. | |
| # --------------------------------------------------------------------------- | |
| gate: | |
| if: | | |
| github.event_name == 'workflow_dispatch' || | |
| (github.event_name == 'issue_comment' && github.event.issue.pull_request && | |
| contains(github.event.comment.body || '', 'As a PR reviewer and CODEOWNER')) || | |
| (github.event_name == 'pull_request_review' && | |
| contains(github.event.review.body || '', 'As a PR reviewer and CODEOWNER')) || | |
| (github.event_name == 'pull_request_review_comment' && | |
| contains(github.event.comment.body || '', 'As a PR reviewer and CODEOWNER')) | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| pull-requests: read | |
| outputs: | |
| proceed: ${{ steps.resolve.outputs.proceed }} | |
| pr-number: ${{ steps.resolve.outputs.pr-number }} | |
| head-sha: ${{ steps.resolve.outputs.head-sha }} | |
| signoff-author: ${{ steps.resolve.outputs.signoff-author }} | |
| signoff-kind: ${{ steps.resolve.outputs.signoff-kind }} | |
| signoff-fetch-cmd: ${{ steps.resolve.outputs.signoff-fetch-cmd }} | |
| steps: | |
| - name: Resolve PR state and sign-off metadata | |
| id: resolve | |
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 | |
| with: | |
| script: | | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| const repoFull = `${owner}/${repo}`; | |
| const eventName = context.eventName; | |
| // Normalize the event payloads (3 comment events + manual dispatch). | |
| let prNumber, author, refId, kind, fetchCmd; | |
| if (eventName === 'issue_comment') { | |
| prNumber = context.payload.issue.number; | |
| author = context.payload.comment.user.login; | |
| refId = context.payload.comment.id; | |
| kind = 'conversation comment'; | |
| fetchCmd = `gh api repos/${repoFull}/issues/comments/${refId} --jq .body`; | |
| } else if (eventName === 'pull_request_review') { | |
| prNumber = context.payload.pull_request.number; | |
| author = context.payload.review.user.login; | |
| refId = context.payload.review.id; | |
| kind = 'review summary'; | |
| fetchCmd = `gh api repos/${repoFull}/pulls/${prNumber}/reviews/${refId} --jq .body`; | |
| } else if (eventName === 'pull_request_review_comment') { | |
| prNumber = context.payload.pull_request.number; | |
| author = context.payload.comment.user.login; | |
| refId = context.payload.comment.id; | |
| kind = 'inline review comment'; | |
| fetchCmd = `gh api repos/${repoFull}/pulls/comments/${refId} --jq .body`; | |
| } else if (eventName === 'workflow_dispatch') { | |
| // Parse a sign-off URL like | |
| // https://github.com/OWNER/REPO/pull/123#issuecomment-456 | |
| // .../pull/123#pullrequestreview-456 | |
| // .../pull/123#discussion_r456 | |
| const url = (context.payload.inputs && context.payload.inputs.comment_url) || ''; | |
| const prMatch = url.match(/\/pull\/(\d+)/); | |
| if (!prMatch) { | |
| core.setOutput('proceed', 'false'); | |
| core.setFailed(`comment_url must contain /pull/<number>: ${url}`); | |
| return; | |
| } | |
| prNumber = parseInt(prMatch[1], 10); | |
| let m; | |
| if ((m = url.match(/#issuecomment-(\d+)/))) { | |
| refId = m[1]; | |
| kind = 'conversation comment'; | |
| fetchCmd = `gh api repos/${repoFull}/issues/comments/${refId} --jq .body`; | |
| const c = await github.rest.issues.getComment({ owner, repo, comment_id: parseInt(refId, 10) }); | |
| author = c.data.user.login; | |
| } else if ((m = url.match(/#pullrequestreview-(\d+)/))) { | |
| refId = m[1]; | |
| kind = 'review summary'; | |
| fetchCmd = `gh api repos/${repoFull}/pulls/${prNumber}/reviews/${refId} --jq .body`; | |
| const r = await github.rest.pulls.getReview({ owner, repo, pull_number: prNumber, review_id: parseInt(refId, 10) }); | |
| author = r.data.user.login; | |
| } else if ((m = url.match(/#discussion_r(\d+)/))) { | |
| refId = m[1]; | |
| kind = 'inline review comment'; | |
| fetchCmd = `gh api repos/${repoFull}/pulls/comments/${refId} --jq .body`; | |
| const rc = await github.rest.pulls.getReviewComment({ owner, repo, comment_id: parseInt(refId, 10) }); | |
| author = rc.data.user.login; | |
| } else { | |
| core.setOutput('proceed', 'false'); | |
| core.setFailed(`comment_url must end with #issuecomment-<id>, #pullrequestreview-<id>, or #discussion_r<id>: ${url}`); | |
| return; | |
| } | |
| } else { | |
| core.setOutput('proceed', 'false'); | |
| return; | |
| } | |
| const { data: pr } = await github.rest.pulls.get({ | |
| owner, repo, pull_number: prNumber, | |
| }); | |
| // Event-triggered: only act on open, non-draft ("ready") PRs. | |
| // Manual dispatch is an explicit override, so allow any PR state. | |
| const ready = eventName === 'workflow_dispatch' | |
| ? true | |
| : (pr.state === 'open' && pr.draft === false); | |
| if (!ready) { | |
| core.info(`PR #${prNumber} is not ready (state=${pr.state}, draft=${pr.draft}); skipping.`); | |
| } | |
| core.setOutput('proceed', ready ? 'true' : 'false'); | |
| core.setOutput('pr-number', String(prNumber)); | |
| core.setOutput('head-sha', pr.head.sha); | |
| core.setOutput('signoff-author', author); | |
| core.setOutput('signoff-kind', kind); | |
| core.setOutput('signoff-fetch-cmd', fetchCmd); | |
| verify: | |
| needs: gate | |
| if: ${{ needs.gate.outputs.proceed == 'true' }} | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| pull-requests: write | |
| issues: write | |
| actions: read | |
| statuses: write | |
| steps: | |
| # SECURITY: this is a privileged job (write perms + secrets) triggered by | |
| # comment events, so it must NOT check out untrusted PR head code — the | |
| # setup step and the MCP server would then execute attacker-controlled | |
| # files. Check out the trusted default branch; all PR content is read | |
| # read-only via the GitHub API (gh / MCP), never from the working tree. | |
| - name: Checkout repository (trusted default branch only) | |
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | |
| with: | |
| fetch-depth: 0 | |
| ref: ${{ github.event.repository.default_branch }} | |
| token: ${{ secrets.CLAUDE_PAT }} | |
| - name: Setup MCP Server | |
| run: | | |
| pip3 install -r .claude/requirements-mcp.txt | |
| mkdir -p /tmp/inferencemax-mcp | |
| - name: Verify sign-off with Claude | |
| uses: anthropics/claude-code-action@v1 | |
| env: | |
| GH_TOKEN: ${{ secrets.CLAUDE_PAT }} | |
| GITHUB_TOKEN: ${{ secrets.CLAUDE_PAT }} | |
| INFERENCEMAX_ROOT: ${{ github.workspace }} | |
| BASH_DEFAULT_TIMEOUT_MS: "1800000" | |
| BASH_MAX_TIMEOUT_MS: "3600000" | |
| with: | |
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | |
| github_token: ${{ secrets.CLAUDE_PAT }} | |
| track_progress: false | |
| allowed_bots: '' | |
| additional_permissions: | | |
| actions: read | |
| settings: | | |
| {"fastMode": true} | |
| claude_args: | | |
| --model 'claude-fable-5' | |
| --mcp-config '{"mcpServers": {"fetch": {"command": "npx", "args": ["-y", "@anthropic-ai/mcp-server-fetch@latest"]}, "inferencemax-repos": {"command": "python3", "args": ["${{ github.workspace }}/.claude/mcp/server.py"], "env": {"INFERENCEMAX_ROOT": "${{ github.workspace }}"}}}}' | |
| --allowedTools "Read,Glob,Grep,WebFetch,mcp__github__*,mcp__github_ci__*,mcp__fetch__*,mcp__inferencemax-repos__*,Bash" | |
| prompt: | | |
| REPO: ${{ github.repository }} | |
| PR NUMBER: ${{ needs.gate.outputs.pr-number }} | |
| PR HEAD SHA: ${{ needs.gate.outputs.head-sha }} | |
| SIGN-OFF AUTHOR: ${{ needs.gate.outputs.signoff-author }} | |
| SIGN-OFF KIND: ${{ needs.gate.outputs.signoff-kind }} | |
| You are an automated merge-gate auditor for InferenceX. | |
| A CODEOWNER (`${{ needs.gate.outputs.signoff-author }}`) just posted the reviewer | |
| sign-off checklist (as a ${{ needs.gate.outputs.signoff-kind }}) that marks | |
| PR #${{ needs.gate.outputs.pr-number }} as ready to merge. Your job is to | |
| INDEPENDENTLY verify the checks below (0-8). Do not trust the reviewer's checkmarks | |
| — re-derive every conclusion from CODEOWNERS, CI runs, the PR diff, the master | |
| configs, and the linked recipe yourself. Be rigorous and specific. The checks encode | |
| the merge standard in `docs/PR_REVIEW_CHECKLIST.md` (read it in the checked-out | |
| default branch — it is the source of truth if wording here and there ever drifts). | |
| Read the exact sign-off body first (especially its "Additional detail section"). | |
| The sign-off was posted as a ${{ needs.gate.outputs.signoff-kind }}, so fetch it with: | |
| ```bash | |
| ${{ needs.gate.outputs.signoff-fetch-cmd }} | |
| ``` | |
| Get the PR metadata and the full diff (this is the "InferenceX PR recipe" you will | |
| compare against later): | |
| ```bash | |
| gh pr view ${{ needs.gate.outputs.pr-number }} --repo ${{ github.repository }} --json title,headRefName,headRefOid,files,body | |
| gh pr diff ${{ needs.gate.outputs.pr-number }} --repo ${{ github.repository }} | |
| ``` | |
| Anchor everything to the pinned head SHA `${{ needs.gate.outputs.head-sha }}` (the | |
| commit that was signed off). First confirm the PR tip has not moved since the gate | |
| ran: if `headRefOid` from the command above differs from the pinned SHA, the head | |
| advanced mid-verification — assess the recipe at the PINNED SHA (e.g. | |
| `gh api repos/${{ github.repository }}/commits/${{ needs.gate.outputs.head-sha }}` and | |
| the files at that SHA), and note in your comment that a fresh sign-off is needed for | |
| the new commit. This keeps Check 3 (recipe) consistent with Checks 1-2. | |
| ## Check 0 — The sign-off author is a CODEOWNER for the changed files | |
| The sign-off must come from a CODEOWNER for what the PR changes. Read | |
| `.github/CODEOWNERS` (in the checked-out default branch) and, for each changed file, | |
| find its owners via last-matching-pattern-wins (the LAST matching line wins; owners do | |
| not accumulate, so `configs/nvidia-master.yaml @a @b` overrides `* @org/team`). | |
| Then decide: | |
| - A path whose most-specific owner is a SPECIFIC line (named users/team): the signer | |
| `@${{ needs.gate.outputs.signoff-author }}` must be one of those owners (listed | |
| directly, or a member of that team). | |
| - A path whose ONLY owner is the broad catch-all (`* @org/team`): satisfied by ANY | |
| recognized CODEOWNER. So if the signer is listed anywhere in CODEOWNERS (e.g. they | |
| own one of the specific files in this PR), the catch-all paths are covered too — | |
| the catch-all is a default, not a per-file gatekeeper. | |
| - Be decisive and DO NOT hard-fail on unreadable team membership. The bot's token | |
| often can't read org team membership (403/404) — that is not a failure. If the | |
| signer is clearly a CODEOWNER (listed in CODEOWNERS for any changed path, or for a | |
| specific changed file), treat Check 0 as PASS. Do not write "if they're a member | |
| then OK, otherwise…" hedging. | |
| - FAIL only on a genuine mismatch: the signer is not a CODEOWNER for a SPECIFIC | |
| (non-catch-all) changed path — e.g. an AMD owner signing an NVIDIA-only change. | |
| Name the path and its real owners in one line. | |
| ## Check 1 — A passing sweep + evals ran on a commit IN this PR | |
| The merge standard (and InferenceX's own reuse gate) requires a green full sweep — | |
| including evals — on a commit that is CURRENTLY part of this PR. A sweep that ran on a | |
| commit later rebased/force-pushed out does NOT count: at merge, `merge_with_reuse.sh` | |
| → `validate_reusable_run` (in `utils/find_reusable_sweep_run.py`) rejects any source | |
| whose `head_sha` is not in `GET /pulls/<n>/commits`. So the whole question collapses | |
| to one fact: does a commit still in this PR carry green, executed sweep/eval checks? | |
| The cleanest way to answer is the check-runs attached to each in-PR commit — you do | |
| NOT need to list `run-sweep.yml` runs or parse reuse logs. | |
| - Get the PR's current commit SHAs: | |
| ```bash | |
| gh api repos/${{ github.repository }}/pulls/${{ needs.gate.outputs.pr-number }}/commits \ | |
| --paginate --jq '.[].sha' | |
| ``` | |
| - For each of those SHAs, list its check-runs and look at the sweep/eval jobs: | |
| ```bash | |
| gh api repos/${{ github.repository }}/commits/<sha>/check-runs --paginate \ | |
| --jq '.check_runs[] | {name, conclusion}' | |
| ``` | |
| The per-config benchmark/eval check-runs are named like `single-node 1k1k /`, | |
| `single-node 8k1k /`, and `eval /`. A commit satisfies validation only if the actual | |
| executed jobs — the `eval /` AND `single-node */` check-runs — have conclusion | |
| `success`. `skipped` does NOT count (it means a skip/reuse run that executed nothing | |
| on that commit); `failure` does not count. | |
| IMPORTANT: do NOT rely on `collect-evals` — it is an aggregator that can report | |
| `success` even when every underlying `eval /` job was `skipped` (it just aggregated | |
| an empty set). Always key off the per-config `eval /` and `single-node */` jobs. | |
| - PASS if ANY commit currently in the PR has green (success, non-skipped) `single-node */` | |
| AND `eval /` check-runs. Remember the run id behind a passing `eval /` check (its | |
| `details_url` contains `/actions/runs/<run_id>/...`) — Check 2 needs it. | |
| - Otherwise FAIL. State the ROOT ISSUE plainly and keep it actionable: | |
| "No passing sweep/eval was found on any commit in this PR." | |
| Do NOT write a confusing message like "it technically passed but the commit isn't in | |
| the PR" — a sweep that ran on a rebased-out commit is irrelevant to the reviewer, so | |
| don't lead with it. The fix the author needs is simply: run (or re-anchor via | |
| `/reuse-sweep-run`) a passing full sweep on a commit currently in this PR. You may | |
| add an offending run/SHA as a short supporting detail AFTER the root-issue line. | |
| ## Check 2 — Evals actually pass (accuracy), on that in-PR commit's run | |
| For the commit that passed Check 1, confirm the eval numbers are real and meet the bar | |
| — not just that the job is green: | |
| - Take the run id behind the passing `eval /` / `collect-evals` check-run (from its | |
| `details_url`) and download its eval results: | |
| ```bash | |
| gh run download <RUN_ID> --repo ${{ github.repository }} -p 'eval_results_*' -D ./evals || \ | |
| gh run download <RUN_ID> --repo ${{ github.repository }} -p 'eval_*' -D ./evals | |
| find ./evals -name '*.json' | head | |
| ``` | |
| - Read the aggregated eval JSON / the run's "Eval Summary" step summary and confirm | |
| accuracy is present and meets the expected bar for the model, and that the run used | |
| the same inference-engine image as this PR's config. FAIL if evals are | |
| skipped/failed/empty/below bar, or the image differs — say exactly which. | |
| ## Check 3 — Recipe linked AND complete (SINGLE-NODE recipes only) | |
| APPLICABILITY — read this first: the recipe-link requirement covers SINGLE-NODE | |
| recipes only, because the official upstream recipe sources (vLLM recipes, SGLang | |
| cookbook) publish single-node serve commands. Disaggregated / multi-node | |
| submissions have NO recipe-link requirement. If the PR's benchmark changes are | |
| exclusively multi-node/disagg — files under `benchmarks/multi_node/**` (including | |
| `srt-slurm-recipes/**`), and/or master-config entries with `multinode: true` or | |
| `disagg: true`, and/or disagg frameworks (`dynamo-trt`, `dynamo-sglang`, | |
| `sglang-disagg`, vLLM disagg, ATOM/ATOMesh disagg) — report this check as | |
| `N/A — disaggregated/multi-node submission; the recipe-link requirement applies to | |
| single-node recipes only` and DO NOT fail it. A sign-off note like "this is a | |
| disagg submission, no recipe update required" is a legitimate statement of that | |
| fact, not a violation. If the PR touches BOTH single-node and multi-node recipes, | |
| apply (a)/(b) below to the single-node portion only. | |
| The InferenceX "recipe" for this PR = the files it changes under | |
| `benchmarks/single_node/**` plus its entry in `configs/*-master.yaml`. The merge | |
| standard is: the community must be able to reproduce this benchmark from a public | |
| recipe. | |
| - (a) LINK PRESENT: The sign-off's "Additional detail section" MUST contain a link to | |
| the corresponding recipe — a PR/commit in | |
| `https://github.com/vllm-project/recipes` or | |
| `https://github.com/sgl-project/sglang` (cookbook under `docs_new`), or the | |
| published recipe page (`https://recipes.vllm.ai/` or | |
| `https://docs.sglang.io/cookbook/...`). If no such link is present, FAIL. | |
| - (b) MAJOR SERVER ARGS MATCH: Fetch the linked recipe (use the `fetch` MCP tool or | |
| WebFetch; for a recipe PR, read its diff via `gh pr diff` against that repo if | |
| accessible) and compare it to this PR's launch command. The recipe only needs to | |
| match the MAJOR, deployment-defining server args — NOT every flag, and explicitly | |
| NOT the knobs that are specific to InferenceX benchmark/harness tuning. | |
| MAJOR (must match — these define the model, parallelism, precision, and which | |
| kernels run, so they determine the perf profile): | |
| - model / model-path, hardware/SKU | |
| - parallelism: TP / EP / DP / PP and DP-attention flags | |
| (`--enable-dp-attention`, `--enable-dp-lm-head`, etc.) | |
| - quantization and kv-cache dtype | |
| - kernel-selection backends: `--attention-backend`, `--moe-runner-backend`, | |
| `--enable-flashinfer-allreduce-fusion` and similar | |
| - other flags that materially change the served model or its throughput | |
| INFERENCEX-SPECIFIC (do NOT require a match — list as informational only, never a | |
| failure): per-lane sweep tuning and harness plumbing such as | |
| `--scheduler-recv-interval`, `--chunked-prefill-size`, `--disable-piecewise-cuda-graph`, | |
| `SGLANG_RADIX_FORCE_MISS` and similar env toggles, concurrency / sequence-length | |
| sweep ranges, ports, result filenames, and image tag/version. | |
| FAIL only if a MAJOR arg in this PR is missing from (or contradicts) the recipe; | |
| list exactly those. Treat the InferenceX-specific diffs as expected and mention them | |
| only as a brief informational note, not as blockers. If a flag's effect is | |
| equivalent to a recipe default (e.g. quantization auto-detected from an FP4 model), | |
| say so and do not count it against the recipe. | |
| - Note: a bare "recipes are already similar to the official ones" claim WITHOUT a | |
| link does not pass this workflow's standard — a link is required. | |
| ## Check 4 — Reuse-sweep command explicitly posted | |
| The supported merge path for an approved PR is reuse (`utils/merge_with_reuse.sh`), | |
| which can only find a run to reuse if an authorized maintainer has explicitly posted | |
| the `/reuse-sweep-run` command as a PR comment. A green sweep alone is NOT enough — the | |
| reuse command must be on record so the merge actually consumes that sweep rather than | |
| silently re-running it. Verify it directly from the PR's comments: | |
| - List the PR's conversation comments and look for the reuse command at the start of a | |
| comment line (it may be bare `/reuse-sweep-run` or pin a run id, | |
| `/reuse-sweep-run <run_id>`): | |
| ```bash | |
| gh api repos/${{ github.repository }}/issues/${{ needs.gate.outputs.pr-number }}/comments \ | |
| --paginate --jq '.[] | {user: .user.login, association: .author_association, body: .body}' | |
| ``` | |
| - PASS only if at least one such `/reuse-sweep-run` comment exists AND its author is | |
| authorized — `author_association` is `OWNER`, `MEMBER`, or `COLLABORATOR` (the same | |
| authorization the reuse path itself enforces). A `/reuse-sweep-run` from an | |
| unauthorized author does not count. | |
| - FAIL if no `/reuse-sweep-run` comment is present, or the only such comment is from an | |
| unauthorized author. State the root issue plainly: "No authorized `/reuse-sweep-run` | |
| command has been posted on this PR" and remind the reviewer that an authorized | |
| maintainer must comment `/reuse-sweep-run` before this PR can be merged via reuse. | |
| ## Check 5 — Sign-off uses the LATEST checklist template | |
| The first item of the checklist has the reviewer affirm they used the latest version | |
| of `docs/PR_REVIEW_CHECKLIST.md`. Verify it instead of trusting it: read the template | |
| in `docs/PR_REVIEW_CHECKLIST.md` (checked-out default branch) and compare its items | |
| against the sign-off body. | |
| - PASS if every item in the current template has a corresponding checked (`[x]`) item | |
| in the sign-off. Match items semantically — minor wording drift is fine; a missing | |
| ITEM is not. | |
| - FAIL if the sign-off is missing current-template items (stale copy) or left items | |
| unchecked without an explanation in the additional detail section. Name the | |
| missing/unchecked items and link the current template. | |
| ## Check 6 — Upstream vLLM/SGLang images, and engine-first ordering | |
| The checklist makes upstream engine images a HARD guideline: on established hardware, | |
| vLLM/SGLang submissions must run images published by the upstream projects, not | |
| vendor forks. Established (NOT "new hardware") SKUs: NVIDIA H100, H200, B200, B300, | |
| GB200, GB300; AMD MI300X, MI325X, MI355X. | |
| Identify each master-config entry this PR adds/changes (in `configs/*-master.yaml`) | |
| and read its `framework:`, `runner:`, and `image:` fields. | |
| - (a) UPSTREAM IMAGE: for entries with `framework: vllm`, the image must come from the | |
| upstream vLLM Docker Hub org (https://hub.docker.com/u/vllm) — in the master configs | |
| that looks like `vllm/vllm-openai:<tag>` or `vllm/vllm-openai-rocm:<tag>`. For | |
| `framework: sglang`, it must come from the upstream org | |
| (https://hub.docker.com/u/lmsysorg) — `lmsysorg/sglang:<tag>` or | |
| `lmsysorg/sglang-rocm:<tag>` (digest-pinned `@sha256:...` variants are fine). | |
| Vendor/private forks — e.g. `rocm/sgl-dev`, `rocm/vllm-dev`, `ghcr.io#...`, or any | |
| non-`vllm/`/non-`lmsysorg/` repo — FAIL on the established SKUs above unless the | |
| sign-off's additional detail section documents a genuine exception: truly new | |
| hardware (e.g. MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8) or a new model | |
| architecture that upstream vLLM/SGLang does not fundamentally support yet (as | |
| backed by vLLM/SGLang community maintainers). Name the offending image and the | |
| missing justification when you FAIL. (Note: some entries write the registry | |
| separator as `#`, e.g. `nvcr.io#nvidia/...` — treat `#` as `/`.) | |
| - (b) ENGINE-FIRST ORDERING: if this PR adds a config entry for a NON-vLLM/SGLang | |
| framework (`framework:` of `trtllm`, `atom`, dynamo variants, etc. — images like | |
| `rocm/atom*`, `nvcr.io...tensorrt-llm...`), check whether `configs/*-master.yaml` | |
| already contains a vLLM or SGLang entry for the same model (`model-prefix`) and SKU | |
| (`runner`). If none exists and no exception is documented in the sign-off, FAIL — | |
| vLLM/SGLang submissions must land before additional frameworks. Otherwise PASS with | |
| the matching entry named. | |
| - N/A if the PR changes no master-config entries (state that in one line). | |
| ## Check 7 — No benchmark hacks that change the model architecture | |
| Verify from the PR diff (server args in `benchmarks/**` and master-config changes) | |
| that nothing alters the model architecture or reduces its FLOPs — e.g. `--hf-overrides` | |
| that skip the indexer every N layers on a model that doesn't natively support it, | |
| trimming layers/experts/heads, or otherwise skipping computation. The rule: making the | |
| SAME computation run faster is fair game; FLOPs at lower precision is fine when evals | |
| pass; REMOVING model-architecture FLOPs is not. Optimizations should be ones used in | |
| production by accuracy-sensitive customers. | |
| - Scan for architecture-override knobs: `--hf-overrides`, `hf_overrides`, | |
| `--json-model-override-args`, config-editing `sed`/`jq` on the model files, etc. If | |
| present, determine whether the override changes computed FLOPs vs the model's native | |
| config and whether the model natively supports that mode. | |
| - FAIL with the exact flag/value if architecture FLOPs are reduced without native | |
| model support. PASS in one line otherwise; N/A if the PR touches no server args. | |
| ## Check 8 — Speculative-decoding configs benchmark through chat templates | |
| If this PR adds or changes a speculative-decoding config (MTP / EAGLE / draft-model | |
| flags such as `--speculative-config`, `--speculative-algorithm`, `spec-decode`, config | |
| names ending in `-mtp`), verify the benchmark client exercises the model through its | |
| chat template (e.g. chat-completions style endpoint/backend rather than raw | |
| completions) so the acceptance-length distribution matches real-world traffic. | |
| - FAIL if a spec-decode benchmark drives raw completions with no chat template — | |
| name the config and script line. | |
| - N/A if the PR has no speculative-decoding changes. | |
| ## Verdict and output | |
| Decide PASS only if Checks 0-8 ALL pass (a check reported as `N/A` counts as a pass — | |
| keep the `N/A — <reason>` row so the reviewer sees it was considered). Post EXACTLY ONE summary comment on | |
| PR #${{ needs.gate.outputs.pr-number }} using `gh pr comment`. Start the comment with | |
| the hidden marker so reruns are identifiable: | |
| `<!-- codeowner-signoff-verify sha=${{ needs.gate.outputs.head-sha }} -->` | |
| Before posting, list the PR's comments and, if a prior verification comment with this | |
| marker already exists for THIS head SHA, do not post a duplicate — update your | |
| assessment only if the conclusion changed. | |
| KEEP IT TIGHT — a busy reviewer should get it in ~15 seconds. Not a novel, not a | |
| single terse line either. Rules: | |
| - First line after the marker: the overall verdict as a markdown header, with the | |
| verdict word in bold and flanked by three status emojis on each side — EXACTLY: | |
| on pass: `## ✅✅✅ **Verdict: PASS** ✅✅✅` | |
| on fail: `## ❌❌❌ **REJECTED** ❌❌❌` | |
| - Then ONE short line per check, each STARTING with its status emoji so pass/fail is | |
| scannable at a glance: | |
| `✅ Check N (<name>): PASS — <brief reason>` | |
| `❌ Check N (<name>): FAIL — <root issue>` | |
| `➖ Check N (<name>): N/A — <reason>` | |
| Spend words only on the checks that fail. | |
| - State conclusions, don't narrate your process. No multi-paragraph explanations, no | |
| restating the checklist, no hedging ("if X then maybe Y" — make the call). Link the | |
| run/recipe instead of describing it. | |
| - If everything is to standard: post the verdict header + the nine one-line rows | |
| (with the green run URL). Do NOT @-mention anyone on a pass. | |
| - If anything is NOT to standard: the verdict header must be immediately followed by a | |
| line that @-mentions the sign-off author as `@${{ needs.gate.outputs.signoff-author }}` | |
| with the blocking summary. Then the per-check lines, each failing one led by its root | |
| issue (e.g. "No passing sweep/eval on any commit in this PR") with the supporting | |
| link after. | |
| Use no emojis anywhere in the comment other than the ✅ / ❌ / ➖ status emojis | |
| specified above. Use only facts you verified. If a required artifact or run is | |
| inaccessible, say so explicitly rather than assuming pass. | |
| # Publish the verdict as a commit status on the PR head SHA so it can act | |
| # as a required check. Deterministic: reads the verdict comment posted | |
| # above (matched by the sha-pinned marker) and maps "Verdict: PASS" to | |
| # success, anything else — including a missing verdict — to failure. | |
| # Runs on always() so a crashed verify step still reports failure rather | |
| # than leaving the status stuck at "expected". | |
| - name: Publish verdict as commit status on PR head | |
| if: always() | |
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 | |
| env: | |
| HEAD_SHA: ${{ needs.gate.outputs.head-sha }} | |
| PR_NUMBER: ${{ needs.gate.outputs.pr-number }} | |
| with: | |
| script: | | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| const headSha = process.env.HEAD_SHA; | |
| const prNumber = parseInt(process.env.PR_NUMBER, 10); | |
| const marker = `<!-- codeowner-signoff-verify sha=${headSha} -->`; | |
| const comments = await github.paginate(github.rest.issues.listComments, { | |
| owner, repo, issue_number: prNumber, per_page: 100, | |
| }); | |
| const verdicts = comments.filter(c => c.body && c.body.includes(marker)); | |
| const latest = verdicts[verdicts.length - 1]; | |
| let state, description, target_url; | |
| if (!latest) { | |
| state = 'failure'; | |
| description = 'No verification verdict was posted for this commit'; | |
| target_url = `https://github.com/${owner}/${repo}/actions/runs/${context.runId}`; | |
| } else if (latest.body.includes('**Verdict: PASS**')) { | |
| state = 'success'; | |
| description = 'CODEOWNER sign-off independently verified'; | |
| target_url = latest.html_url; | |
| } else { | |
| state = 'failure'; | |
| description = 'Sign-off verification rejected - see verdict comment'; | |
| target_url = latest.html_url; | |
| } | |
| await github.rest.repos.createCommitStatus({ | |
| owner, repo, sha: headSha, state, description, | |
| context: 'codeowner-signoff-verify', target_url, | |
| }); | |
| core.info(`codeowner-signoff-verify=${state} on ${headSha}`); |