Enforce deprecated model review / 强制审阅已弃用模型

[adibarra]

Summary

• Require CODEOWNER sign-offs to check current MODELS.md and confirm that submissions do not target deprecated models, scenarios, or model-scenario combinations.
• Add independent verifier Check 7 for deprecated model and scenario submissions, including effective-date handling.
• Update the Chinese recipe explanation to require merged upstream documentation, matching the English checklist requirement.

Validation

• Parsed .github/workflows/codeowner-signoff-verify.yml with python3 and PyYAML.
• Confirmed verifier checks are contiguous from 0 through 11.

中文说明

• 要求 CODEOWNER 签署时查阅最新版 MODELS.md，确认提交未使用已弃用的模型、场景或模型场景组合。
• 新增独立验证器 Check 7，用于验证已弃用模型和场景的提交，并处理生效日期。
• 更新中文 recipe 说明，要求上游文档已合并，与英文审阅清单要求保持一致。

验证

• 使用 python3 和 PyYAML 解析 .github/workflows/codeowner-signoff-verify.yml。
• 已确认验证器检查项从 0 到 11 连续编号。

[claude]

🟡 The PR strengthened the single-node recipe requirement in the copy-paste template (now requires the upstream vLLM/SGLang recipe PR to be merged, not just opened), but left item 11 of the Chinese 各条目中文对照说明 explanation list with the old wording (recipe only needs to be '相似'/similar, and if not, merely '开了 PR'/opened a PR). This PR directly edited that list to renumber items 7-13, so the stale content should be updated to match the new MERGED requirement.

Extended reasoning...

The bug: This PR tightens the single-node recipe checklist item in the copy-paste template (present verbatim in both PR_REVIEW_CHECKLIST.md and the English template block inside PR_REVIEW_CHECKLIST_zh.md) from 'similar to the official recipes, and if not, a PR has been opened' to 'documented in the official recipes, and the corresponding upstream PR verified as MERGED — open/draft/closed-without-merge does not satisfy this requirement.' This is a real, deliberate strengthening of the bar reviewers must apply.

Where it breaks: docs/PR_REVIEW_CHECKLIST_zh.md also contains a supplementary '各条目中文对照说明' (item-by-item Chinese explanation) section that exists specifically to help Chinese-reading reviewers interpret the English template. This PR directly edited that section to insert a new item 7 (for the MODELS.md deprecated-model check) and renumbered the old items 7-12 to 8-13. However, the content of the recipe item — renumbered from old #10 to new #11 (line 52) — was carried over unchanged: it still reads '已确认单节点 recipe 与官方 ... 相似；如果不相似，已确认在 ... 开了 PR' (verified the recipe is similar; if not, a PR was opened). It does not mention 'documented' or the new MERGED requirement at all.

Why nothing else catches this: The template code-fence block itself (the artifact CI's codeowner-signoff-verify.yml keys off of, and what reviewers literally paste into their approval comment) is English-verbatim in both files and is correctly updated — so the enforceable, machine-checked text is consistent. The problem is confined to the human-facing explanatory prose that a Chinese-speaking CODEOWNER would read to understand what the checkbox actually requires. Nothing validates that this prose matches the template it explains, so the PR was free to renumber the list (which it did, correctly, for items 7-13) while missing that item 11's substance also needed to change.

Impact: A Chinese-reading CODEOWNER reviewing a submission with single-node vLLM/SGLang recipes would read item 11, conclude the bar is merely 'similar, and if not similar, a PR must have been opened somewhere,' and sign off accordingly — even though the template checkbox they are attesting to now requires the upstream PR be verified MERGED. This directly undercuts the intent of this PR's own change to the template (before this PR, template and explanation agreed with the weaker 'similar/opened' language; after this PR, they diverge).

Step-by-step reproduction of the inconsistency:

1. Open docs/PR_REVIEW_CHECKLIST_zh.md after this PR.
2. In the '模板' code fence, read the recipe bullet: it requires the recipe be documented in vLLM recipes/SGLang cookbook AND the upstream PR be linked and verified MERGED (explicitly: open/draft/closed-without-merge insufficient).
3. Scroll to '各条目中文对照说明', item 11 (line 52): it says the recipe need only be '相似' (similar), and if not similar, only that a PR was '开了' (opened) — with no mention of 'documented' or 'MERGED'.
4. A reviewer reading only the explanation (which is the section's entire purpose) would approve an upstream PR that is merely open, contradicting the template's actual (and newly strengthened) requirement.

Fix: Update item 11's text to mirror the new template bullet, e.g.: '已确认此 PR 中的每个单节点 recipe 均已记录在官方 vLLM recipes 和/或 SGLang cookbook 中；已在对应仓库中链接上游 PR，并确认其已被 合并（MERGED）——处于 open、draft 或未合并关闭状态的上游 PR 不满足此要求。若相应 recipe 已发布，已在下方 Additional detail section 中链接已发布的 recipe/cookbook 页面。'

Severity: The authoritative, CI-triggering artifact (the English-verbatim template block) is correct and consistent between the EN and ZH files, so this is not a functional/CI break — it's a documentation-quality inconsistency confined to an explanatory aid. Marking as nit: real and worth fixing before merge since it directly undermines this PR's own stated goal of tightening the recipe requirement, but it does not block the enforceable mechanism.

[claude]

🟡 This PR adds a new CODEOWNER sign-off item requiring reviewers to check MODELS.md for deprecated models/scenarios, but doesn't update .github/codeowner-signoff-verify-prompt.md to add a matching independent check. Per AGENTS.md's checklist↔verifier sync rule, new policy items are expected to get their own verifier check (ideally in the same PR); right now the CI verifier's Check 5 only confirms the sign-off checkbox is ticked, not that the deprecated-model claim is independently true.

Extended reasoning...

The bug: docs/PR_REVIEW_CHECKLIST.md (and its zh mirror) gains a new sign-off line: "Verified against the current MODELS.md that this PR does not submit a deprecated model, scenario, or model-scenario combination." This is a substantive new policy item, not a wording tweak. .github/codeowner-signoff-verify-prompt.md, however, is untouched by this PR — it still only defines Checks 0-10 (its own header says "INDEPENDENTLY verify the checks below (0-10)" and the verdict section says "Decide PASS only if Checks 0-10 ALL pass," matching "the eleven one-line rows").

Why this matters — the documented sync rule: AGENTS.md (line 86, "Checklist ↔ sign-off verifier sync") spells out exactly this scenario: "Whenever docs/PR_REVIEW_CHECKLIST.md is updated — an item added, removed, or materially reworded — agents are allowed and expected to update the verifier prompt to match, ideally in the same PR... The verifier's Check 5 already compares sign-offs against the live checklist file, so stale sign-off templates are caught automatically — but a new or removed policy item needs its own check logic added to / removed from the workflow prompt." The verifier prompt's own header comment reiterates: "Keep the checks here in sync with docs/PR_REVIEW_CHECKLIST.md, per the sync rule in AGENTS.md."

What actually happens today: Check 5 (lines ~207-217 of the verifier prompt) only confirms that "every item in the current template has a corresponding checked ([x]) item in the sign-off" — i.e., it checks that a box exists and is ticked for the new MODELS.md line. It does not independently re-derive the claim the way Checks 6, 7, 9, and 10 do for their respective checklist items (e.g. re-checking configs, diffs, or docker image sources against the actual PR contents). So a CODEOWNER could tick the new box without having actually checked MODELS.md, and the automated CI gate — whose whole purpose per the PR title "Enforce deprecated model review" is to backstop human review — would still PASS.

Step-by-step proof:

1. A submission PR targets a model/scenario combination that MODELS.md lists as deprecated.
2. The CODEOWNER sign-off comment ticks every checklist box, including the new "Verified against the current MODELS.md..." item, without actually cross-referencing MODELS.md.
3. The GitHub Action runs .github/codeowner-signoff-verify-prompt.md against the sign-off comment.
4. Check 5 passes because the box is checked and matches the current template text.
5. No other check (0-10) inspects MODELS.md or deprecated-model status at all.
6. Verdict: PASS. The deprecated submission proceeds despite the stated goal of the PR.

Fix: Add a new Check (e.g. Check 11) to .github/codeowner-signoff-verify-prompt.md that independently cross-references the PR's model/scenario/hardware combination against MODELS.md's deprecated list, mirroring how Checks 6/7/9/10 independently verify their respective items rather than just checking a box.

Note on severity: I considered marking this normal since it defeats the PR's own stated enforcement goal at the automated-gate layer, but AGENTS.md frames the verifier-prompt update as "ideally in the same PR" — a soft expectation, not a hard requirement — and Check 5 still forces a human reviewer to affirmatively tick the box, so the policy isn't silently unenforced; it's enforced by the human sign-off and can gain independent CI verification as a fast follow-up. Nothing breaks or regresses at merge time. Treating this as a nit reflects the repo's own convention (an allowed/expected practice, not a blocking requirement) while still flagging the gap for the maintainers to close.