Cookbook Review PR

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

1. gh pr view <N> --repo sgl-project/sglang --json title,body,files,author,baseRefName,headRefName,commits,reviews
2. gh pr diff <N> --repo sgl-project/sglang
3. gh pr list --repo sgl-project/sglang --state open --search "<model name>" (duplicate check)
4. Run every checklist item against the diff.
5. Output per-file verdicts + overall recommendation.

Checklist

1. File hygiene

• A cookbook PR should only touch: docs/src/snippets/configs/<vendor>/*.jsx (config + benchmarks), docs/cookbook/**/*.mdx, docs/docs.json, docs/cookbook/<category>/intro.mdx (vendor card), docs/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.jsx should NOT change in a model-add PR (adding a model is data-only). Engine edits = a separate axis/feature PR (see cookbook-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) — use obj.key === undefined.
• supportedHardware ⊆ HARDWARE_CATALOG (in _deployment.jsx) ∪ config.hardware. A model-specific GPU the shared catalog lacks must be declared in config.hardware ({id,label,vram,vendor}), not added to the engine catalog — unless the platform is genuinely shared, which is an engine change the PR body has to argue (and the cookbook-add-model hardware table moves with it). A new catalog entry is inert for other pages: each filters the catalog by its own supportedHardware.
• placeholders declares every {{KEY}} used in curl or any cell.
• modelNames covers every cell (by hw|variant|quant triple or variant|quant pair).
• strategies count matches the page's operating points — 1 recipe → a single balanced; 2 → low-latency + high-throughput; 3 → the full trio. Tiers apply per (hw × variant × quant) combination: a single-recipe combination must park under its semantically honest tier (clear slant → that tier, e.g. a workstation card under low-latency; no slant → balanced, e.g. a CPU platform) — flag a no-slant recipe parked under low-latency/high-throughput. Mixed unions like [low-latency, balanced, high-throughput] with per-selection greying are fine. Also flag model-specific ids (e.g. mtp), and flag an INVERTED speculative mapping — the deterministic default is MTP/spec-decoding ON → low-latency, OFF → high-throughput (at saturation the draft+verify overhead outweighs the speedup); the reverse needs an explicit maintainer-confirmed justification in the PR. The MDX strategy bullets describe serving semantics in the DSv4 style (single-user chat / typical multi-user / batch jobs), not internal toggles.
• dockerImages covers the hw ids that have cells (else users hit the :dev fallback); a hw|quant key (resolved before the plain hw) is valid when one quant on a shared GPU needs a different image (e.g. an FP4 dev build) — don't flag those.
• multiNodeHints present ONLY for hw whose fabric needs manual NIC env (e.g. gb200 NVL72) — NOT every multi-N hw (standard-IB DeepEP / Marlin multi-node don't need it). Hints render above BOTH run modes, so flag one that reads as docker-only ("add these docker run flags") — docker run flags belong in the hardware entry's multiNodeDockerFlags, which the engine emits into the Docker command itself.
• github.cookbookModel is set to the model's HF id (<hf-org>/<model-slug>). The issue template's model field is a free-form input prefilled from this value; if the config omits the github block, the engine falls back to deepseek-ai/deepseek-v4 and the page's submissions get mislabeled.
• playgroundFeatures is opt-OUT: the general axes ship on every cookbook by default (attention TP/CP/DP-Attn, moe backend+EP for MoE models, parsers, speculative, pdDisagg, hicache) — flag a missing general axis unless the model genuinely cannot use it. Model-specific axes only where applicable (MegaMoE backend + megamoeQuant only on Blackwell MoE, gated by requiresHw; hisparse only DSA-style). Knobs that are meaningless for a subset of variants/hw are disabled with a reason, not silently live (e.g. MoE knobs greyed on dense variants). No empty/stub axes.
• 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 supportedHardware id (from the catalog or config.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 match has EXACTLY the 5 keys (hw, variant, quant, strategy, nodes).
• env / flags are flat literals (only {{PLACEHOLDER}} subst) — no shared commonFlags reference (Mintlify won't inline it).
• NO --nnodes / --node-rank / --dist-init-addr literals in multi-node cells (the renderer injects them from match.nodes).
• NO literal --host / --port — use {{HOST_IP}} / {{PORT}}.
• NO --reasoning-parser / --tool-call-parser in any cell — parsers are a Playground-only feature added on top of the base command (DSv4 convention); flag any cell that bakes them in.
• Accuracy-degrading flags in cells — runtime quant below the checkpoint (e.g. MegaMoE W4A4 — DSv4 gates it behind the Playground's megamoeQuant) and lossy --kv-cache-dtype (e.g. fp8_e4m3 over a higher-precision-KV checkpoint): flag for explicit maintainer confirmation. Output quality should be exactly what the quant chip declares, so absent a recorded sign-off in the PR (e.g. carried verbatim from a measured legacy recipe's default command), request the flag move to Playground/tips.
• Flag order: --model-path first (an optional --trust-remote-code may precede it — the DSv4 cells do), then parallelism, then MoE, then tuning, --host/--port last (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's match tuple corresponds to a real cell.
• accuracyLabels is present whenever the benchmarks carry accuracy data — the engine ships NO default eval set; without it the accuracy rows silently don't render. defaultAccuracy / per-cell accuracy / benchmarkCommands.accuracy keys all ∈ config.accuracyLabels.
• A benchmark's quantization must match a variant actually listed — (BF16) on a model that only released FP8/FP4 is a factual bug.
• benchmarkCommands.speed is python3 -m sglang.bench_serving (the workload), separate from the sglang serve deploy command, and should carry --flush-cache: bench_serving's random prompts are deterministic, so a warm rerun hits the radix cache and inflates throughput — speed numbers are measured cache-cold.
• sglang_version is a real build the author ran (a release, or dev/nightly) — not a guessed/placeholder value (no leftover 0.0.0).
• Latency percentile: config.latencyPercentile (default "P50", or "Mean") matches the percentile the TTFT/TPOT values actually are — the card renders TTFT (<pct>). A benchmarks entry may carry its own latencyPercentile to override the page value per cell (entry → config → "P50"): on a P50 page, kept legacy Mean cells must set it — a sglang_version tag alone doesn't convey the percentile. ("Mean" is temporary — legacy data is being re-measured to P50.)
• Throughput convention: tokens_per_sec_per_gpu is stored as total (in+out)/GPU = output tok/s/GPU × (isl+osl)/osl, shown by the card as-is. Flag output-only values.
• Consistent accuracy harness across entries: every value under one accuracyLabels column must be produced by the SAME harness — flag a page that, say, measures one platform's GSM8K with a historical raw completion harness and another's with sgl-eval run gsm8k --num-examples 1319 and shows both as one "GSM8K %" (the scores aren't comparable). Either standardize on one harness (matching benchmarkCommands.accuracy) or require an explicit per-entry note. Common when folding a second contributor's measurements (e.g. an AMD/ROCm PR) into the page.

5. Doc ↔ config parity (the #1 finding)

• Any sglang serve command 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.