Skip to content

CI: improve PR review plan comment format#338

Merged
QG-phy merged 3 commits into
deepmodeling:mainfrom
QG-phy:review-governance-setup
Jun 3, 2026
Merged

CI: improve PR review plan comment format#338
QG-phy merged 3 commits into
deepmodeling:mainfrom
QG-phy:review-governance-setup

Conversation

@QG-phy
Copy link
Copy Markdown
Collaborator

@QG-phy QG-phy commented Jun 3, 2026

Summary

  • add a --format github-comment output mode for scripts/ci/pr_review_plan.py
  • make the visible PR review plan summary bilingual for faster maintainer scanning
  • keep detailed risk areas, local commands, and hold conditions behind collapsible sections
  • document the difference between CLI output and GitHub comment output

Validation

  • printf "docs/index.rst\n" | python3 scripts/ci/pr_review_plan.py --stdin
  • printf "docs/index.rst\n" | python3 scripts/ci/pr_review_plan.py --stdin --format github-comment
  • printf "dptb/utils/argcheck.py\n" | python3 scripts/ci/pr_review_plan.py --stdin --format github-comment
  • python3 scripts/ci/check_repository_hygiene.py

Note

This PR intentionally does not modify .github/workflows/pr_review_plan_comment.yml. The workflow needs to call --format github-comment for the new PR-page format to appear automatically.

Summary by CodeRabbit

  • New Features

    • Added a GitHub-comment output format for PR review plans and a CLI flag to switch between detailed and comment formats; comment output is bilingual (EN/ZH).
  • Documentation

    • Updated PR review plan docs with format options, examples (including GitHub comment preview), clarified default vs shorter comment behavior, folded bilingual summary guidance, and noted possible appended Evidence section and revised comment permissions.
  • Chores

    • Updated CI workflows to emit/comment the GitHub-format plan and to ignore additional workflow/CI paths for unit test triggers.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

This advisory review plan was generated from changed file names using trusted base-branch code.

DeePTB PR Review Plan

Risk level: Medium
Changed files: 4

Touched Risk Areas

  • GitHub Actions and CI (Medium): .github/workflows/pr_review_plan_comment.yml, .github/workflows/unit_test.yml
  • Documentation (Low): docs/maintenance/pr_review_plan.md
  • Maintenance governance (Low): docs/maintenance/pr_review_plan.md, scripts/ci/pr_review_plan.py

Required Maintainer Focus

  • GitHub Actions and CI: trigger scope, required checks, dependency installation, and runtime noise
  • Documentation: stale commands, stale paths, config consistency, and Sphinx warnings
  • Maintenance governance: review guidance consistency and low-noise maintainer workflow

Suggested AI Review

  • Run the Maintainer Review Prompt from docs/maintenance/ai_review_prompts.md.
  • Run the Test Gap Review Prompt from docs/maintenance/ai_review_prompts.md.

Suggested Checks

  • python scripts/ci/check_repository_hygiene.py
  • uv run python -m sphinx -b html docs docs/_build/pr-review-plan-html
  • uv run pytest ./dptb/tests -m smoke
  • uv run pytest ./dptb/tests -m "not slow"

Hold Conditions To Check

  • CI failed and the failure is not explained.
  • The PR scope is unclear or mixes unrelated changes.
  • AI review found a plausible correctness issue that was not fixed or explicitly waived.

Notes

  • This plan is advisory and does not replace maintainer judgment.
  • Use the higher risk level when the PR body and changed files disagree.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR extends the PR review plan script to support a GitHub-comment-friendly bilingual output format. It adds Chinese labels for risks and areas, introduces shared rendering helpers, implements a new GitHub-comment renderer with folded detail sections, and adds a --format CLI option to choose between the existing CLI format and the new GitHub-comment format. Documentation is updated to describe the new format and usage.

Changes

GitHub-Comment Format and Bilingual Rendering

Layer / File(s) Summary
Bilingual labels and shared rendering helpers
scripts/ci/pr_review_plan.py (lines 24–51, 293–304, 335–380)
Adds RISK_LABELS_ZH and AREA_NAMES_ZH constants for Chinese translations, ai_prompt_suggestions_bilingual() for risk-level-aware bilingual AI prompt suggestions, and format_area() / risk_reasons() helpers for consistent per-area bullet formatting and bilingual "why/risk source" reason assembly.
Refactored CLI renderer
scripts/ci/pr_review_plan.py (lines 394–401)
Updates render_plan() to use the new format_area() helper for "Touched Risk Areas" bullets and changes the unclassified-file label to reference risk_map.md.
GitHub-comment renderer
scripts/ci/pr_review_plan.py (lines 424–514)
Adds render_github_comment_plan() to generate a bilingual Markdown template for GitHub comments with risk summary, "Why" reasons, recommended review items from bilingual AI suggestions, conditional HIGH-risk guidance, and multiple collapsible <details> sections for risk areas, human focus, and local commands.
Format selection and CLI dispatch
scripts/ci/pr_review_plan.py (lines 530–535, 549–552)
Extends parse_args() with a --format option (default cli, alternate github-comment) and updates main() to dispatch to either render_plan() or render_github_comment_plan() based on the chosen format.
Documentation of new format and usage
docs/maintenance/pr_review_plan.md (lines 23–30, 40–51, 61, 78–83)
Documents the shorter GitHub-comment format with --format github-comment CLI example, clarifies workflow permissions (pull-requests: write), explains Evidence section and comment update behavior, describes bilingual GitHub comment summary content with folded details, and adds a GitHub-comment preview command example.
Workflow wiring for github-comment output
.github/workflows/pr_review_plan_comment.yml (lines 44, 56)
Update workflow step to run the script with --format github-comment and add a Chinese descriptor string to the comment body array.
CI workflow trigger ignore update
.github/workflows/unit_test.yml (lines 7–10)
Extend on.pull_request.paths-ignore to include additional workflow files and scripts/ci/**, so PRs touching only these paths will not trigger the unit test workflow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete; it lacks required sections from the template such as Scope, DeePTB Impact Area, Risk and Compatibility, Tests, AI Assistance, and Merge Decision. Complete the PR description by filling out all required template sections: add Scope, check DeePTB Impact Area, specify Risk level and compatibility impacts, document test validation, declare AI assistance, and provide Merge Decision.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: improving the PR review plan comment format with a new bilingual GitHub comment output mode.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
docs/maintenance/pr_review_plan.md (1)

36-41: 💤 Low value

Consider clarifying that the workflow needs to be updated to use the new format.

The documentation describes the GitHub comment format and its benefits (lines 29, 61), but the workflow code (Context snippet 3) still runs python scripts/ci/pr_review_plan.py --stdin without the --format github-comment flag. While the PR objectives explicitly note this, readers of the documentation might assume the GitHub comment format is already live on PRs.

Consider adding a note like:

The comment workflow currently uses the default CLI format. To adopt the GitHub comment format, the workflow should add `--format github-comment` to the script invocation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/maintenance/pr_review_plan.md` around lines 36 - 41, Update the comment
workflow to invoke the new GitHub-comment format by modifying the job step that
runs the script in `.github/workflows/pr_review_plan_comment.yml` (Context
snippet 3): change the command that calls `python scripts/ci/pr_review_plan.py
--stdin` to include the `--format github-comment` flag so it runs `python
scripts/ci/pr_review_plan.py --stdin --format github-comment` (or equivalent
ordering), ensuring the workflow emits the documented GitHub comment output
format instead of the default CLI format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/maintenance/pr_review_plan.md`:
- Around line 36-41: Update the comment workflow to invoke the new
GitHub-comment format by modifying the job step that runs the script in
`.github/workflows/pr_review_plan_comment.yml` (Context snippet 3): change the
command that calls `python scripts/ci/pr_review_plan.py --stdin` to include the
`--format github-comment` flag so it runs `python scripts/ci/pr_review_plan.py
--stdin --format github-comment` (or equivalent ordering), ensuring the workflow
emits the documented GitHub comment output format instead of the default CLI
format.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e358e814-f404-4568-854c-0bc73f23e58e

📥 Commits

Reviewing files that changed from the base of the PR and between 4163ce9 and 2e1e0b1.

📒 Files selected for processing (2)
  • docs/maintenance/pr_review_plan.md
  • scripts/ci/pr_review_plan.py

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/unit_test.yml (1)

7-10: ⚡ Quick win

Review paths-ignore scope in .github/workflows/unit_test.yml for scripts/ci/**

scripts/ci/ currently contains only scripts/ci/check_repository_hygiene.py and scripts/ci/pr_review_plan.py. However, scripts/ci/pr_review_plan.py constructs uv run pytest ./dptb/tests ... command strings (used to generate which test commands other workflows run), so PRs touching only scripts/ci/** will skip unit_test.yml despite affecting test selection/execution behavior elsewhere. If this is intended, document the constraint (that scripts/ci/** must not include test-execution logic) or narrow the ignore to the specific safe files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/unit_test.yml around lines 7 - 10, The paths-ignore in
unit_test.yml currently excludes 'scripts/ci/**', which hides changes to
scripts/ci/pr_review_plan.py (and thus affects test selection/execution via
generated "uv run pytest ..." command strings); update the paths-ignore scope to
either (a) remove 'scripts/ci/**' so any change to scripts/ci/* triggers
unit_test.yml, or (b) narrow the ignore to the safe file(s) only (e.g.,
'scripts/ci/check_repository_hygiene.py') so edits to
scripts/ci/pr_review_plan.py will not be skipped, and/or add documentation in
unit_test.yml commenting the intended constraint; locate references to
paths-ignore in unit_test.yml and adjust accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/unit_test.yml:
- Around line 7-10: The paths-ignore in unit_test.yml currently excludes
'scripts/ci/**', which hides changes to scripts/ci/pr_review_plan.py (and thus
affects test selection/execution via generated "uv run pytest ..." command
strings); update the paths-ignore scope to either (a) remove 'scripts/ci/**' so
any change to scripts/ci/* triggers unit_test.yml, or (b) narrow the ignore to
the safe file(s) only (e.g., 'scripts/ci/check_repository_hygiene.py') so edits
to scripts/ci/pr_review_plan.py will not be skipped, and/or add documentation in
unit_test.yml commenting the intended constraint; locate references to
paths-ignore in unit_test.yml and adjust accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c20ae3b2-d82e-490a-bb01-4f866ec7aaf2

📥 Commits

Reviewing files that changed from the base of the PR and between b23eaed and 30656d4.

📒 Files selected for processing (1)
  • .github/workflows/unit_test.yml

@QG-phy QG-phy merged commit 4cfcd93 into deepmodeling:main Jun 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant