diff --git a/.github/workflows/pr_review_plan_comment.yml b/.github/workflows/pr_review_plan_comment.yml index 1fe92cf3..e877bcbc 100644 --- a/.github/workflows/pr_review_plan_comment.yml +++ b/.github/workflows/pr_review_plan_comment.yml @@ -41,7 +41,7 @@ jobs: - name: Generate review plan run: | - python scripts/ci/pr_review_plan.py --stdin < changed-files.txt > pr-review-plan.md + python scripts/ci/pr_review_plan.py --stdin --format github-comment < changed-files.txt > pr-review-plan.md - name: Comment review plan uses: actions/github-script@v7 @@ -53,6 +53,7 @@ jobs: const body = [ marker, "This advisory review plan was generated from changed file names using trusted base-branch code.", + "此审查计划由受信任的 base 分支代码根据变更文件名生成,仅作为维护者辅助。", "", plan, ].join("\n"); diff --git a/.github/workflows/unit_test.yml b/.github/workflows/unit_test.yml index df127dfa..7cdf717e 100644 --- a/.github/workflows/unit_test.yml +++ b/.github/workflows/unit_test.yml @@ -4,6 +4,10 @@ on: pull_request: paths-ignore: - 'docs/**' + - '.github/workflows/pr_review_plan.yml' + - '.github/workflows/pr_review_plan_comment.yml' + - '.github/workflows/repository_hygiene.yml' + - 'scripts/ci/**' jobs: build: diff --git a/docs/maintenance/pr_review_plan.md b/docs/maintenance/pr_review_plan.md index 2d7c047d..a137a7ad 100644 --- a/docs/maintenance/pr_review_plan.md +++ b/docs/maintenance/pr_review_plan.md @@ -20,6 +20,14 @@ git diff --name-only main...HEAD | python scripts/ci/pr_review_plan.py --stdin The output is Markdown and can be pasted into a PR's AI Assistance or Merge Decision notes. +For a shorter PR-page comment format, use: + +```bash +git diff --name-only main...HEAD | python scripts/ci/pr_review_plan.py --stdin --format github-comment +``` + +The default CLI format is intentionally detailed. The GitHub comment format is optimized for maintainer scanning: the visible summary is bilingual, while detailed file lists, local commands, and hold conditions are kept in collapsible sections. + ## GitHub Automation DeePTB has two advisory workflows for this plan: @@ -29,16 +37,18 @@ DeePTB has two advisory workflows for this plan: The comment workflow is designed for fork PRs. It checks out the trusted base commit from the main repository, reads changed file names through the GitHub API, then runs the base-branch copy of `scripts/ci/pr_review_plan.py`. It must not checkout or execute code from the fork branch. +The comment workflow may also add an Evidence section based on GitHub check status. That evidence is owned by the workflow, not by `pr_review_plan.py`, because the script only knows changed file names. + Required permissions for the comment workflow are intentionally narrow: ```yaml permissions: contents: read - pull-requests: read + pull-requests: write issues: write ``` -`issues: write` is needed because PR timeline comments use the issues comments API. The workflow updates the existing bot comment using a hidden marker instead of creating a new comment on every PR update. +`issues: write` is needed because PR timeline comments use the issues comments API. `pull-requests: write` is needed for repositories where GitHub requires both issue-comment and pull-request write scopes for PR timeline comments. The workflow updates the existing bot comment using a hidden marker instead of creating a new comment on every PR update. ## How To Read The Output @@ -48,6 +58,7 @@ permissions: - **Suggested AI review** points to the minimum recommended prompts. - **Suggested checks** lists likely local commands for this PR. - **Hold conditions** names situations that should stop merge until resolved or explicitly waived. +- **GitHub comment summary** shows bilingual risk, reason, and review focus, with details folded by default. The plan is advisory. When the PR body, maintainer judgment, or scientific impact suggests higher risk than the file map, use the higher risk level. @@ -64,3 +75,9 @@ Docs-only changes should normally stay low risk: ```bash printf "docs/index.rst\n" | python scripts/ci/pr_review_plan.py --stdin ``` + +GitHub comment preview: + +```bash +printf "dptb/utils/argcheck.py\n" | python scripts/ci/pr_review_plan.py --stdin --format github-comment +``` diff --git a/scripts/ci/pr_review_plan.py b/scripts/ci/pr_review_plan.py index 50d4da1c..86b40364 100644 --- a/scripts/ci/pr_review_plan.py +++ b/scripts/ci/pr_review_plan.py @@ -21,6 +21,34 @@ class Risk(IntEnum): Risk.HIGH: "High", } +RISK_LABELS_ZH = { + Risk.LOW: "低", + Risk.MEDIUM: "中", + Risk.HIGH: "高", +} + +AREA_NAMES_ZH = { + "AtomicDataDict contract": "AtomicDataDict 契约", + "Orbital indexing": "轨道索引", + "Hamiltonian expansion": "Hamiltonian 展开", + "Model assembly": "模型构建", + "Config schema": "配置 schema", + "Training behavior": "训练行为", + "Loss behavior": "loss 行为", + "Embedding and prediction": "embedding 与 prediction", + "Dataset backends": "数据集后端", + "CLI entrypoints": "CLI 入口", + "Postprocess and export": "后处理与导出", + "Plugins": "插件", + "GitHub Actions and CI": "GitHub Actions 与 CI", + "Tests": "测试", + "Examples and configs": "示例与配置", + "Documentation": "文档", + "Issue templates": "issue 模板", + "Packaging metadata": "打包元数据", + "Maintenance governance": "维护治理", +} + @dataclass(frozen=True) class AreaRule: @@ -262,6 +290,18 @@ def ai_prompt_suggestions(risk: Risk) -> list[str]: ] +def ai_prompt_suggestions_bilingual(risk: Risk) -> list[str]: + if risk == Risk.LOW: + return [ + "Optional: run the Test Gap Review Prompt if examples, docs commands, or test markers changed. / " + "可选:如果示例、文档命令或测试 marker 有变更,运行测试缺口审查 prompt。", + ] + return [ + "Run the Maintainer Review Prompt. / 运行维护者审查 prompt。", + "Run the Test Gap Review Prompt. / 运行测试缺口审查 prompt。", + ] + + def maintainer_focus(risk: Risk, matched: list[tuple[AreaRule, list[str]]], paths: list[str]) -> list[str]: focus = [f"{rule.name}: {rule.focus}" for rule, _ in matched] if any(path.startswith("dptb/data/") for path in paths) and any( @@ -292,6 +332,52 @@ def hold_conditions(risk: Risk) -> list[str]: return holds +def format_area(rule: AreaRule, files: list[str], limit: int = 5) -> str: + preview = ", ".join(files[:limit]) + suffix = "" if len(files) <= limit else f", ... ({len(files)} files)" + return f"- **{rule.name}** ({RISK_LABELS[rule.risk]}): {preview}{suffix}" + + +def risk_reasons( + risk: Risk, + matched: list[tuple[AreaRule, list[str]]], + unmatched: list[str], + paths: list[str], +) -> list[str]: + reasons: list[str] = [] + for rule, _ in matched: + if rule.risk == Risk.HIGH: + zh_name = AREA_NAMES_ZH.get(rule.name, rule.name) + reasons.append(f"{rule.name} changed. / {zh_name} 有变更。") + + if risk == Risk.HIGH and not reasons: + reasons.append("High-risk areas changed. / 高风险区域有变更。") + + for rule, _ in matched: + if rule.risk == Risk.MEDIUM and len(reasons) < 4: + zh_name = AREA_NAMES_ZH.get(rule.name, rule.name) + reasons.append(f"{rule.name} changed. / {zh_name} 有变更。") + + if has_any(paths, prefixes=("examples/",)): + reasons.append("Examples and configs changed. / 示例和配置有变更。") + if has_any(paths, prefixes=("docs/",), exact=("README.md",)): + reasons.append("Documentation changed. / 文档有变更。") + if unmatched: + reasons.append( + "Some files are unclassified by current `risk_map.md`. / " + "部分文件尚未被当前风险地图覆盖。" + ) + + if not reasons: + reasons.append("No mapped high-risk area was detected. / 未发现已映射的高风险区域。") + + deduped: list[str] = [] + for reason in reasons: + if reason not in deduped: + deduped.append(reason) + return deduped[:5] + + def render_plan(paths: list[str]) -> str: risk, matched, unmatched = classify(paths) lines: list[str] = [ @@ -305,16 +391,14 @@ def render_plan(paths: list[str]) -> str: if matched: for rule, files in matched: - preview = ", ".join(files[:5]) - suffix = "" if len(files) <= 5 else f", ... ({len(files)} files)" - lines.append(f"- **{rule.name}** ({RISK_LABELS[rule.risk]}): {preview}{suffix}") + lines.append(format_area(rule, files)) else: lines.append("- No mapped DeePTB risk area matched. Review scope manually.") if unmatched: preview = ", ".join(unmatched[:8]) suffix = "" if len(unmatched) <= 8 else f", ... ({len(unmatched)} files)" - lines.append(f"- **Unmapped files**: {preview}{suffix}") + lines.append(f"- **Unclassified by current risk_map.md**: {preview}{suffix}") sections = ( ("Required Maintainer Focus", maintainer_focus(risk, matched, paths)), @@ -337,6 +421,97 @@ def render_plan(paths: list[str]) -> str: return "\n".join(lines) + "\n" +def render_github_comment_plan(paths: list[str]) -> str: + risk, matched, unmatched = classify(paths) + reasons = risk_reasons(risk, matched, unmatched, paths) + focus = maintainer_focus(risk, matched, paths) + checks = suggested_checks(risk, paths) + holds = hold_conditions(risk) + + lines: list[str] = [ + "## DeePTB PR Review Plan / DeePTB PR 审查计划", + "", + f"**Risk / 风险等级: {RISK_LABELS[risk]} ({RISK_LABELS_ZH[risk]})** · " + f"**Changed files / 变更文件: {len(paths)}**", + "", + "## Why / 风险来源", + "", + ] + lines.extend(f"- {reason}" for reason in reasons) + + lines.extend( + [ + "", + "## Recommended Review / 建议审查重点", + "", + ] + ) + lines.extend(f"- {item}" for item in ai_prompt_suggestions_bilingual(risk)) + if risk == Risk.HIGH: + lines.append( + "- Focus human review on config/API/data/checkpoint compatibility. / " + "人工重点看配置、API、数据和 checkpoint 兼容性。" + ) + else: + lines.append( + "- Confirm the changed behavior matches the PR scope. / " + "确认变更行为和 PR 范围一致。" + ) + + lines.extend( + [ + "", + "
", + "Detailed risk areas", + "", + ] + ) + if matched: + lines.extend(format_area(rule, files) for rule, files in matched) + else: + lines.append("- No mapped DeePTB risk area matched. Review scope manually.") + if unmatched: + preview = ", ".join(unmatched[:20]) + suffix = "" if len(unmatched) <= 20 else f", ... ({len(unmatched)} files)" + lines.append(f"- **Unclassified by current risk_map.md**: {preview}{suffix}") + lines.extend(["", "
"]) + + lines.extend( + [ + "", + "
", + "Human review focus", + "", + ] + ) + lines.extend(f"- {item}" for item in focus) + lines.extend(["", "
"]) + + lines.extend( + [ + "", + "
", + "Local commands and hold conditions", + "", + "Suggested local commands:", + "", + ] + ) + lines.extend(f"- `{check}`" for check in checks) + lines.extend(["", "Hold conditions:", ""]) + lines.extend(f"- {item}" for item in holds) + lines.extend( + [ + "", + "
", + "", + "_Advisory only. / 仅作为审查辅助。_", + ] + ) + + return "\n".join(lines) + "\n" + + def parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser( description="Generate an advisory DeePTB PR review plan from changed files." @@ -352,6 +527,12 @@ def parse_args() -> argparse.Namespace: action="store_true", help="Read newline-separated changed files from stdin.", ) + parser.add_argument( + "--format", + choices=("cli", "github-comment"), + default="cli", + help="Output format. Default keeps the detailed CLI-style Markdown.", + ) return parser.parse_args() @@ -365,7 +546,10 @@ def main() -> int: ) return 2 - print(render_plan(paths), end="") + if args.format == "github-comment": + print(render_github_comment_plan(paths), end="") + else: + print(render_plan(paths), end="") return 0