Prhub

#35597 [misc] Add a comment style rule to .claude/rules

原始 PR 作者 hnyls2002 合并时间 2026-08-20 09:52 文件变更 2 提交数 4 评论 0 代码增减 +184 / -279

执行摘要

新增注释风格规则并裁剪 CI 权重校验模块冗余注释

PR body 指出需要“Codifies what a comment has to carry to earn its place, so new comments state constraints the code cannot show instead of restating it.” 即把注释的价值标准写进仓库规则,让新注释表达代码无法展示的约束,而不是复述代码。规则文档还定义了“删除测试”:删掉注释后,熟悉仓库的人能否从代码加全局搜索恢复该事实,能恢复则注释不应存在。

值得快速精读规则文件本身,“删除测试”和 Keep / Delete / Move 三分类对任何希望控制注释噪音的团队都有借鉴价值。ci_weight_validation.py 的 diff 适合作为规则落地的示范案例,但不必深入阅读其实现。若团队对注释风格敏感,建议在后续 PR 中观察规则执行的一致性与争议点。

讨论亮点

该 PR 全程无 review 评论与讨论,由作者 hnyls2002 自行合并。有价值的设计讨论体现在 commit 演进中:第一个 commit 只加规则,第二个 commit 将作用域从全局压缩到代码路径(避免误伤 md 等文档),第三个 commit 补齐 Doxygen 覆盖并对 docstring 回归测试,第四个 commit 用真实文件验证规则可操作性。这相当于作者对规则进行了四轮自我评审。

实现拆解

  1. 新增规则文件 .claude/rules/comment-style.md(+164 行)。
    规则声明适用于 /.py、.cu、.cuh、.cpp、.h、.rs 下的 #//、Python docstring 和 C/C++ Doxygen 块,确立“删除测试”为判断标准:注释被删除后,若熟悉仓库的人仍能从周围代码和 grep 恢复同一事实,则注释应删除。

  2. 规则正文按 Keep / Delete / Move 三类给出处置依据。Keep 覆盖跨边界约束、命名无法承载的单位与布局、魔法数字出处、带可验证引用与退役条件的工作区、决策型契约;Delete 覆盖复述代码、逐步旁白、命名已表达的信息;Move 则要求把真实但属于别处的事实迁移到常量、文档、issue 或 commit message。

  3. 将规则应用到 python/sglang/srt/model_loader/ci_weight_validation.py(+20/-279)。模块级 docstring 从 15 行长描述压缩为 4 行用途说明,_get_per_run_marker_dir、_get_per_run_marker_path、_read_per_run_marker、_write_per_run_marker 等函数的 docstring 全部删除,仅在 _get_per_run_marker_dir 中保留一句跨 runner 缓存隔离的关键注释;所有逻辑分支、返回值和异常处理保持不变。
  4. 演进验证:4 个 commit 依次完成“添加规则 → 将规则作用域限定到代码路径 → 覆盖 Doxygen 并回归测试 docstring → 将规则应用到实际文件”,未新增自动化测试,规则通过 .claude/rules 配置对 AI 编码助手和人工 review 生效。
文件 模块 状态 重要度
.claude/rules/comment-style.md 代码规范 added 5.46
python/sglang/srt/model_loader/ci_weight_validation.py 模型加载 modified 7.09

关键符号

_get_per_run_marker_dir _get_per_run_marker_path _read_per_run_marker _write_per_run_marker

关键源码片段

python/sglang/srt/model_loader/ci_weight_validation.py style

规则落地的示范文件:删除 279 行冗余 docstring 与注释,仅保留关键约束说明,验证规则可操作性。

def _get_per_run_marker_dir() -> str:
    # Markers are per CI run; sharing them across runners leaks cache state.
    # 原 docstring 已删除:它的信息可从上下文与变量名推断,属于 Delete 类。
    base_dir = os.environ.get("RUNNER_TEMP", os.environ.get("TMPDIR", "/tmp"))
    marker_dir = os.path.join(base_dir, "sglang_ci_offline_markers")
    os.makedirs(marker_dir, exist_ok=True)
    return marker_dir
​
​
def _get_per_run_marker_path(snapshot_dir: str) -> Optional[str]:
    if not snapshot_dir or not os.path.isdir(snapshot_dir):
        return None
​
    normalized_dir = os.path.realpath(snapshot_dir).rstrip("/")
    dir_hash = hashlib.sha256(normalized_dir.encode("utf-8")).hexdigest()[:12]
​
    marker_dir = _get_per_run_marker_dir()
    return os.path.join(marker_dir, f"{dir_hash}.json")

评论区精华

没有提炼出高价值讨论线程

当前评论区没有形成足够清晰的争议点或结论,后续有更多讨论时会体现在这里。

风险与影响

主要风险是注释与 docstring 的大规模删除(-279 行)可能降低新手对 CI 权重校验模块的理解速度,但规则明确要求保留跨边界约束等关键事实,且本次删除的都是可从代码结构恢复的信息,逻辑未变。另一个风险是规则文件未经多人 review 即合入,Keep / Delete / Move 的边界在真实场景下可能引发争议,后续 PR 可能因注释判定标准产生摩擦。规则随 .claude/rules 生效,不影响运行时行为,回归风险极低。

对开发者:后续提交的 Python / CUDA / C++ / Rust 注释将按“删除测试”标准被审视,能减少低信息量注释;对 AI 辅助编码:Claude Code 会读取该规则,自动生成的注释也会遵循同一风格;对系统运行:完全无影响,ci_weight_validation.py 逻辑未变;对团队流程:新增了一类代码规范文档,需要维护者共识支撑其长期效力。

注释大规模删除 规则未经多人评审

关联 Issue

未识别关联 Issue

当前没有检测到明确关联的 Issue 链接,后续同步到相关引用后会出现在这里。

完整报告

参与讨论