Skip to content

[Klaud Cold] codeowners: own MODELS.md and MODELS_zh.md / 为 MODELS.md 与 MODELS_zh.md 指定代码所有者 - #2532

Merged
functionstackx merged 3 commits into
mainfrom
feat/codeowners-models-md
Aug 7, 2026
Merged

[Klaud Cold] codeowners: own MODELS.md and MODELS_zh.md / 为 MODELS.md 与 MODELS_zh.md 指定代码所有者#2532
functionstackx merged 3 commits into
mainfrom
feat/codeowners-models-md

Conversation

@functionstackx

Copy link
Copy Markdown
Collaborator

Adds named CODEOWNERS for MODELS.md and MODELS_zh.md.

MODELS.md    @ankur-singh @chunfangamd @InferenceX/core
MODELS_zh.md @ankur-singh @chunfangamd @InferenceX/core

Why

MODELS.md is the source of truth for which scenarios are active per model and for the deprecation calendar, but until now it only matched the * @InferenceX/core catch-all — so a change to a retirement date needed no vendor-side review.

That matters more than it used to:

Owner choice

The union of the two master-config reviewer sets, because the file spans both vendors:

Owner Also owns
@ankur-singh configs/nvidia-master.yaml
@chunfangamd configs/amd-master.yaml
@InferenceX/core repo catch-all

All three already appear in .github/CODEOWNERS, so no new handles are introduced.

Both files carry the identical list because AGENTS.md requires an English doc and its _zh counterpart to be edited in the same PR — split ownership would let the pair drift.

Notes

  • Placed after the master-config rules and before experimental/operatorx/. CODEOWNERS is last-match-wins; these are exact-path rules and no later pattern matches them, so the placement is safe.
  • No behavior change outside review routing. One file, one hunk.

中文说明

MODELS.mdMODELS_zh.md 指定具名代码所有者。

背景MODELS.md 是各模型启用场景与弃用时间表的权威来源,但此前仅由 * @InferenceX/core 兜底匹配,修改退役日程无需厂商侧评审。当前该文件的影响已显著提升:退役执行 PR 完全依据其内容(#2493 据此移除 54 个配置项,#2527 再移除 17 个);且 .github/codeowner-signoff-verify-prompt.md 的弃用模型检查(Check 3/7)直接读取该文件,已据此驳回 #2512#2513 —— 修改 MODELS.md 可能改变其他 PR 的合入门槛。

所有者选择:取两份主配置评审人的并集,因为该文件同时覆盖两家厂商 —— @ankur-singh(NVIDIA 主配置)、@chunfangamd(AMD 主配置)、@InferenceX/core(仓库兜底)。三者均已存在于 .github/CODEOWNERS,未引入新账号。两个文件使用完全相同的所有者列表,因为 AGENTS.md 要求英文文档与其 _zh 对应文件在同一 PR 中同步修改,所有权若分离会导致两者脱节。

说明:规则置于主配置规则之后、experimental/operatorx/ 之前;CODEOWNERS 采用「最后匹配优先」,这两条为精确路径规则且其后无可匹配的模式,放置位置安全。除评审路由外无任何行为变更,仅涉及一个文件、一处改动。

MODELS.md is the PoR for which scenarios are active per model and for the
deprecation schedule, but it only matched the `* @InferenceX/core` catch-all,
so a change to the retirement calendar needed no vendor review. Recent
enactment PRs turn on exactly what it says, and the sign-off verifier's
deprecated-model check reads it directly, so give it named owners.

Owners are the union of the two master-config reviewer sets, since the file
spans both vendors: @Ankur-singh (NVIDIA master), @chunfangamd (AMD master),
and @InferenceX/core. Both files carry the same list because AGENTS.md requires
the English doc and its `_zh` counterpart to be edited in the same PR.

No behavior change outside review routing.

中文:为 MODELS.md 与 MODELS_zh.md 指定代码所有者。该文件是各模型启用场景与弃用
时间表的 PoR,此前仅由 `* @InferenceX/core` 兜底匹配,修改退役日程无需厂商侧评审。
鉴于近期的退役执行 PR 完全依据该文件,且签核校验器的弃用模型检查也直接读取该文件,
故为其指定具名所有者。

所有者取两份主配置评审人的并集(该文件同时覆盖两家厂商):@Ankur-singh(NVIDIA
主配置)、@chunfangamd(AMD 主配置)与 @InferenceX/core。两个文件使用相同的所有者
列表,因为 AGENTS.md 要求英文文档与其 `_zh` 对应文件在同一 PR 中同步修改。

除评审路由外无任何行为变更。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Added new code owners for the MODEL files.
Removed vendor-specific entries from CODEOWNERS for MODEL files.
@functionstackx
functionstackx merged commit 64b3ca7 into main Aug 7, 2026
3 checks passed
@functionstackx
functionstackx deleted the feat/codeowners-models-md branch August 7, 2026 23:54
Comment thread .github/CODEOWNERS Outdated
Comment on lines +9 to +14
# Model support matrix and deprecation notices. Spans both vendors, so the
# AMD and NVIDIA master-config owners review it together. Keep the two files
# on the same owner list: MODELS.md and MODELS_zh.md must be edited in the
# same PR (see the bilingual docs rule in AGENTS.md).
MODELS.md @ankur-singh @chunfangamd @InferenceX/core
MODELS_zh.md @ankur-singh @chunfangamd @InferenceX/core

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new MODELS.md / MODELS_zh.md CODEOWNERS rules lack a leading slash, so per gitignore-style CODEOWNERS matching they'd match a file with that basename at any depth in the repo, not just the root-level docs — unlike the adjacent configs/amd-master.yaml/configs/nvidia-master.yaml rules, which are path-anchored. Using /MODELS.md and /MODELS_zh.md anchors to repo root and matches the convention already used above.

Extended reasoning...

The bug: GitHub's CODEOWNERS file uses gitignore-style pattern matching. A pattern with no slash in it matches a file with that basename at any directory depth in the repo tree, while a pattern containing a slash is anchored relative to the repo root (or to the CODEOWNERS-relative path, if it starts with /). The two new lines added by this PR:

\nMODELS.md @ankur-singh @chunfangamd @InferenceX/core\nMODELS_zh.md @ankur-singh @chunfangamd @InferenceX/core\n

contain no slash at all, so they are basename-matching rules, not path-anchored rules — despite the PR description explicitly (and incorrectly) calling them 'exact-path rules.'

Contrast with the surrounding code: The two rules immediately above these, which this PR uses as its ownership-inheritance model, are both path-anchored because they contain a slash:

\nconfigs/amd-master.yaml @billishyahao @chunfangamd @seungrokj @yctseng0211 @1am9trash\nconfigs/nvidia-master.yaml @ankur-singh @kedarpotdar-nv @InferenceX/core\n

These only ever match the single root-level file at that exact path. The new MODELS.md/MODELS_zh.md rules are the only unanchored patterns in the whole file, breaking the anchoring convention every other specific rule follows.

Why nothing catches this today: There is currently exactly one MODELS.md and one MODELS_zh.md in the repo, both at the root, so the rule happens to only match the intended files right now — there's no active misrouting. But nothing stops that from changing. GitHub applies CODEOWNERS as 'last match wins' across the whole tree; if any subdirectory ever gains a file that happens to be named MODELS.md (a vendored copy, an experimental doc, a generated report, etc. — e.g. experimental/operatorx/MODELS.md), this rule would silently attach to it too, pulling in the vendor-review and deprecation-gating semantics the PR describes (including gating .github/codeowner-signoff-verify-prompt.md Check 7) for a file nobody intended to cover.

Step-by-step proof:

  1. Suppose a future PR adds experimental/operatorx/MODELS.md (e.g. a scoped model-support note for that subsystem).
  2. GitHub CODEOWNERS evaluates patterns top-to-bottom and the last matching pattern in the file wins for a given path.
  3. The bare pattern MODELS.md (no slash) matches any path whose basename is MODELS.md, including experimental/operatorx/MODELS.md.
  4. Since this rule sits after the experimental/operatorx/ @hbarclay rule's neighbourhood in file order... actually it sits before that rule in the current file, but any future bare-pattern rule ordering aside, the core issue is that the pattern is basename-only and will match at unintended depths as soon as such a file exists, regardless of where new rules are placed relative to it.
  5. The result: an unrelated subdirectory file would silently inherit reviewer requirements (and potentially any doc gating written to look at 'MODELS.md changes') intended only for the two root docs — with no error or warning, just quietly wrong ownership.

The fix: Prepend a leading slash to anchor each pattern to the repo root, matching the style already used for the master-config rules:

\n/MODELS.md @ankur-singh @chunfangamd @InferenceX/core\n/MODELS_zh.md @ankur-singh @chunfangamd @InferenceX/core\n

This is a one-character-per-line change, has zero effect on current behavior (since only the root files exist today), and removes the latent basename-matching ambiguity going forward. All four independent verifiers confirmed this reasoning and the practical fix; none of them found a current misrouting, which is why this is filed as a nit rather than a blocking issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant