Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
10 KiB
10 KiB
name, description
| name | description |
|---|---|
| cookbook-review-pr | Review a pull request against the SGLang Cookbook (docs_new/, Mintlify) contribution checklist — the config-driven format (per-model config + benchmarks JSX consumed by the shared _deployment.jsx / _playground.jsx engines). Run with /cookbook-review-pr <PR number>. |
Cookbook Review PR
Fetch the diff, run the checklist, report what you find. The cookbook is config-driven:
shared engines (_deployment.jsx, _playground.jsx) with NO model-specific code; each
model is a data config (+ optional benchmarks) under src/snippets/configs/<vendor>/
plus an MDX page. This checklist targets that layout. Field-schema detail lives in
.claude/skills/cookbook-add-model/references/authoring-reference.md — defer to it rather
than restating.
Usage
/cookbook-review-pr <PR number>
Steps
gh pr view <N> --repo sgl-project/sglang --json title,body,files,author,baseRefName,headRefName,commits,reviewsgh pr diff <N> --repo sgl-project/sglanggh pr list --repo sgl-project/sglang --state open --search "<model name>"(duplicate check)- Run every checklist item against the diff.
- Output per-file verdicts + overall recommendation.
Checklist
1. File hygiene
- A cookbook PR should only touch:
docs_new/src/snippets/configs/<vendor>/*.jsx(config + benchmarks),docs_new/cookbook/**/*.mdx,docs_new/docs.json,docs_new/cookbook/<category>/intro.mdx(vendor card),docs_new/cards/logos/<vendor>.png(new vendor only). Flag stray files (settings.local.json, lockfiles, IDE configs). - Pages must be
.mdx, not.md. Files end with a trailing newline. Check commit history for unrelated commits accidentally included. - Engines untouched:
_deployment.jsx/_playground.jsxshould NOT change in a model-add PR (adding a model is data-only). Engine edits = a separate axis/feature PR (seecookbook-add-model/references/engine-axis.md); review them against that checklist.
2. Config quality (the per-model config)
- Single
export const config = { ... }literal — no function calls, spreads, fragment refs, or IIFE (Mintlify re-evals at hydration →ReferenceError). - No
!(x in y)anywhere (Mintlify AST walker crashes) — useobj.key === undefined. supportedHardware⊆HARDWARE_CATALOG(in_deployment.jsx) ∪config.hardware. A model-specific GPU the shared catalog lacks must be declared inconfig.hardware({id,label,vram,vendor}), not added to the engine catalog.placeholdersdeclares every{{KEY}}used incurlor any cell.modelNamescovers every cell (byhw|variant|quanttriple orvariant|quantpair).dockerImagescovers the hw ids that have cells (else users hit the:devfallback).multiNodeHintspresent ONLY for hw whose fabric needs manual NIC env (e.g.gb200NVL72) — NOT everymulti-Nhw (standard-IB DeepEP / Marlin multi-node don't need it).github.cookbookModelmatches the issue-templatemodeldropdown value.playgroundFeaturesaxes are pruned to what the model supports — no empty/stub axes (themoeaxis's MegaMoE backend option +megamoeQuantblock only on Blackwell MoE, gated byrequiresHw;hisparseonly DSA-style;pdDisagg.routeronly with a PD topology).- No leftover
__TOKEN__— the config was stamped from the template and every placeholder is filled (grep -rn '__[A-Z_]*__'on the new config/benchmarks/MDX returns nothing). - All-hardware considered: every
supportedHardwareid (from the catalog orconfig.hardware) has ≥1 cell OR is a deliberate greyed "coming soon"; AMD was pruned or kept on purpose (not a leftover template family).
3. Cells / 5-dim matrix
- Every cell
matchhas EXACTLY the 5 keys (hw,variant,quant,strategy,nodes). env/flagsare flat literals (only{{PLACEHOLDER}}subst) — no sharedcommonFlagsreference (Mintlify won't inline it).- NO
--nnodes/--node-rank/--dist-init-addrliterals in multi-node cells (the renderer injects them frommatch.nodes). - NO literal
--host/--port— use{{HOST_IP}}/{{PORT}}. - Flag order:
--model-pathfirst, then parallelism, then MoE, then tuning,--host/--portlast (the playground's insert anchors assume this). - TP/memory sanity:
model_weight_GB / (tp × gpu_mem)fits with ~20–30% headroom (BF16 ≈ params×2 GB, FP8 ≈ ×1, FP4 ≈ ×0.5; MoE uses total weight, not active params).
4. Benchmarks
- Each
benchmarks[]entry'smatchtuple corresponds to a real cell. defaultAccuracykeys ∈ACCURACY_LABELS(andbenchmarkCommands.accuracy).- A benchmark's quantization must match a variant actually listed —
(BF16)on a model that only released FP8/FP4 is a factual bug. benchmarkCommands.speedispython3 -m sglang.bench_serving(the workload), separate from thesglang servedeploy command.sglang_versionis a real build the author ran (a release, ordev/nightly) — not a guessed/placeholder value (no leftover0.0.0).
5. Doc ↔ config parity (the #1 finding)
- Any
sglang servecommand shown in MDX prose (config tips, benchmark section) must equal what the engine emits from the corresponding cell — same flags, same order. Drift here is the most common review miss.
6. Commands / port
- Launch uses
sglang serve— flag anypython -m sglang.launch_server/python3 -m sglang.launch_server(deprecated). The engine already emitssglang serve; guard against prose/cells reintroducing the old launcher. - Port
30000everywhere (launch, curl, clientbase_url, bench) — flag8000. Launch port must match client/curl port on the same page.
7. Frontmatter
- Every new MDX page has
title:and a top-leveldescription:(a real one-line value prop, not copied from another vendor) — NOTmetatags.description(non-canonical; the top-level field is what renders as the subtitle and SEO meta — see mintlify-authoring). - No
mode: wideon a model page — it hides the right-hand "On this page" ToC that every other model page has. Leavemodeunset (the Deploy/Playground panels self-cap at 900px, so the default column holds them fine).mode: widebelongs only on categoryintro.mdxgrids. tag: NEWonly for genuine new launches; when one is added, staletag: NEWon older pages should be dropped in the same PR (grep -RlE "^tag: NEW" docs_new/cookbook/).- MDX imports BOTH
DeploymentandPlaygroundfrom/src/snippets/...(absolute). - Deploy heading slugs to
deployment(ordeploy), Playground toplayground— so "↑ Switch base" and "Open the Playground →" scroll. No numbered headings for these two.
8. Navigation & homepage
- New page →
docs_new/docs.jsonupdated: under the right vendor group insidenavigation→ Cookbook → Autoregressive Models, root-relative, no.mdx:cookbook/<category>/<Vendor>/<Model>. - Homepage
<Card href>indocs_new/cookbook/<category>/intro.mdxpoints to the vendor's flagship; new vendors get a new<Card>+ a logo atdocs_new/cards/logos/<vendor>.png— 940×525 RGBA transparent, icon-only (no wordmark), lowercase filename, tracked viagit add -f(*.pngis gitignored repo-wide). Card order matches thedocs.jsonnav order. - Don't change
docs_new/cookbook/intro.mdxfor individual model adds (top-level only).
9. Links & factual
- HuggingFace URLs resolve to a real model. License section matches the actual HF license
(don't copy from another model). Docker images from
lmsysorg/sglang; nosgl-project-dev. The image tag is a real build (a release the author ran, or:dev/nightly) — not a guessed version. - Internal links root-relative, no extension (
/cookbook/.../<Model>); flag.md/.mdxor../-relative links.docs.sglang.iois canonical. - No Google-Drive image links (don't render). Shell placeholders are
export VAR=<value>, not${VAR}(a bash no-op).
9b. MDX authoring (Mintlify) — detail in cookbook-add-model/references/mintlify-authoring.md
- Forbidden syntax: no Docusaurus admonitions (
:::),@site/@theme, GitHub alert blocks (> [!NOTE]), markdown pipe tables (use JSX<table>), inline<details>, or unknown components.<CardGroup>/<Card>only on categoryintro.mdx, not model pages. - Code fences are labeled (e.g.
python Example/bash Command/text Outputafter the opening fence); a fenced block nested inside another uses four backticks outside. - Every runnable invocation block is followed by
**Output Example:**+ atext Outputfenced block (real output, orPending update...only with user acknowledgement). - Reasoning-parser example matches the parser's output shape: separate-field
(
reasoning_content+content) vs inline<think>tags parsed out ofcontent. - No hardcoded sampling params (
temperature/top_p) in sample code (SGLang usesgeneration_config.jsondefaults); listing them in §1 informationally is fine.
10. Quantization rules
- FP4 is Blackwell-only (B200/B300/GB300) — never AMD; AMD FP4 chips must be
disabled. - BF16 / FP8 work on NVIDIA and AMD. FP8 configs adding
--kv-cache-dtype fp8_e4m3should note the accuracy trade-off.
11. Scope
- Changes match the PR title. Flag global changes hiding behind a platform-specific title (e.g. an "H200 FP8" PR that adds a flag to ALL cells). Unmentioned side-fixes belong in the PR body.
12. Duplicate PRs
- Another open PR for the same model? Flag it; compare completeness; note merge-conflict
risk on
docs.json+ the vendor card; flag a superseded older PR by the same author.
13. Build / validate
cd docs_new
mint validate
mint broken-links
Optional: mint dev for a visual smoke test.
14. Reviewer feedback
gh api repos/sgl-project/sglang/pulls/<N>/comments— have prior reviewer requests been addressed? Unresolved requested-changes should be flagged.
15. Grammar & spelling
- Check added/changed prose for typos and grammar (e.g. "recommend" vs "recommended"). Flag each with the exact wrong text + correction.
Output
Per file:
- ✅ PASS
- ⚠️ ISSUE: <what>
- 🔴 BLOCK: <what>
Overall: APPROVE / REQUEST CHANGES / BLOCKED