Skip to content

docs: disclose human review status in pull requests - #717

Open
zywind wants to merge 2 commits into
mainfrom
yunfeng/disclose-human-review
Open

docs: disclose human review status in pull requests#717
zywind wants to merge 2 commits into
mainfrom
yunfeng/disclose-human-review

Conversation

@zywind

@zywind zywind commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add four human-review disclosure levels to the pull request template
  • preselect the safe default and use neutral status lights to make review depth easy to scan
  • instruct agents to report human review accurately without claiming unconfirmed review

Human review

Select exactly one. The default is no human review; uncheck it when choosing another level.

  • ⚪ No human review; automated review only
  • 🟡 Partially reviewed or spot-checked by a human
  • 🔵 Complete diff reviewed by a human
  • 🟢 Complete diff reviewed and verified by a human

Verification performed: Agent compared the final diff with the linked proposal and ran the checks below, and also checked by human

Test plan

  • mise run format-check
  • mise run lock-check
  • mise run test -- attempted; existing vLLM-dependent tests cannot run on macOS because vllm is unavailable
  • mise run typecheck -- existing vllm and pynvml imports are unavailable on macOS

Summary by CodeRabbit

  • Documentation
    • Added a Human review section to the pull request template, including review-status options and verification details.
    • Added guidance for accurately documenting whether human review was completed.

Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
@zywind
zywind requested a review from a team as a code owner August 12, 2026 20:57
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 99f8b8df-6eea-48f3-b8b5-5f540be7f412

📥 Commits

Reviewing files that changed from the base of the PR and between 520fdd5 and 1389fec.

📒 Files selected for processing (2)
  • .github/PULL_REQUEST_TEMPLATE.md
  • AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/PULL_REQUEST_TEMPLATE.md
📜 Recent review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: CI Status
  • GitHub Check: Greptile Review
  • GitHub Check: Analyze (Python)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{md,markdown,py}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings

Files:

  • AGENTS.md
**/*.{md,markdown}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use ## headers to segment markdown sections instead of bold text
Use -- (em-dash) instead of - (hyphen) for asides in markdown

Files:

  • AGENTS.md
AGENTS.md

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

Maintain agent guide with module map and conventions in AGENTS.md

Files:

  • AGENTS.md
**/*.md

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Do not use decorative bold text in body content; use single backticks for inline code and -- for asides. In Python docstrings, use double backticks and MkDocs autorefs rather than Sphinx roles.

Files:

  • AGENTS.md
**/*.{py,sh,yaml,yml,md}

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.

Include SPDX copyright headers in all source files, except files explicitly listed in .copyrightignore.

Files:

  • AGENTS.md
**/*

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.

**/*: Do not commit unless the user asks for a commit or PR work.
When committing, all commits require DCO sign-off and GPG signing.
Always use git commit --signoff --gpg-sign (or -s -S) -- never write the Signed-off-by trailer manually, and never pass --no-gpg-sign.

Files:

  • AGENTS.md

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

  • Refactor suggestion: use for local maintainability problems introduced
    by the diff when they have clear future cost, such as duplicated setup,
    unclear boundaries, over-mocking, avoidable complexity, or opaque test
    helpers.
  • Nitpick: avoid in chill mode. Do not emit formatting, import-order,
    wording, or style-only comments unless automated tools cannot catch the
    issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

  • Major: incorrect generation/training/evaluation behavior, broken
    CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
    cleanup and process-isolation bugs likely to fail CI or production
    runs.
  • Minor: localized bugs, missing focused tests for changed behavior, or
    bad test patterns that weaken regression coverage.
  • Trivial: small cleanup with no behavior impact. Usually suppress in
    chill mode.
  • Info: context only. Avoid unless it helps reviewers understand risk.
    Safe-Synthesizer-specific review focus: - Data ...

Files:

  • AGENTS.md
**/*.{py,sh,yaml,yml,toml,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use mise tasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.

Files:

  • AGENTS.md
🪛 LanguageTool
AGENTS.md

[uncategorized] ~38-~38: The official name of this software platform is spelled with a capital “H”.
Context: ...t, complete the Human review section in .github/PULL_REQUEST_TEMPLATE.md accurately. K...

(GITHUB)

🔇 Additional comments (1)
AGENTS.md (1)

38-39: LGTM!


Walkthrough

The pull request template now records human-review status and verification details. AGENTS.md requires accurate completion of these fields.

Changes

Human review guidance

Layer / File(s) Summary
Review status contract
.github/PULL_REQUEST_TEMPLATE.md, AGENTS.md
The template adds review-status options and a verification field. Contributor guidance prohibits unsupported claims of human review or verification.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested labels: docs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: disclosing human review status in pull requests.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yunfeng/disclose-human-review

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

@github-actions github-actions Bot added area:dev-ex Affects build or dev experience area:ci labels Aug 12, 2026
@coderabbitai coderabbitai Bot added the docs Documentation-only change label Aug 12, 2026
@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds standardized human-review disclosure levels to the pull request template and instructs agents to report only explicitly confirmed human review.

  • Preselects “No human review” as the safe default.
  • Defines partial, complete, and verified human-review levels.
  • Aligns agent guidance with the new disclosure section.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
.github/PULL_REQUEST_TEMPLATE.md Adds a clear, mutually exclusive human-review disclosure section with a conservative default.
AGENTS.md Directs agents to preserve the default unless a human explicitly confirms a higher review level.

Reviews (2): Last reviewed commit: "docs: improve human review status select..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/PULL_REQUEST_TEMPLATE.md (1)

9-19: 📐 Maintainability & Code Quality | 🔵 Trivial

Confirm the required CODEOWNERS review before merge.

If this change is still automated-only, do not merge it yet. .github/PULL_REQUEST_TEMPLATE.md is covered by the repository's CODEOWNERS rule for @NVIDIA-NeMo/safe-synthesizer-maintainers. The No human review; automated review only option does not replace that maintainer approval.

As per path instructions: changes under .github/ require maintainer review via CODEOWNERS.

Source: Path instructions


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 125a2e6b-453f-4e2e-b652-a3424dc18063

📥 Commits

Reviewing files that changed from the base of the PR and between edaa85f and 520fdd5.

📒 Files selected for processing (2)
  • .github/PULL_REQUEST_TEMPLATE.md
  • AGENTS.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{md,markdown,py}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings

Files:

  • AGENTS.md
**/*.{md,markdown}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use ## headers to segment markdown sections instead of bold text
Use -- (em-dash) instead of - (hyphen) for asides in markdown

Files:

  • AGENTS.md
AGENTS.md

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

Maintain agent guide with module map and conventions in AGENTS.md

This project loads local developer preferences from @AGENTS.local.md. You MUST read this file if it exists and give its instructions top priority.

Files:

  • AGENTS.md
**/*.md

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Do not use decorative bold text in body content; use single backticks for inline code and -- for asides. In Python docstrings, use double backticks and MkDocs autorefs rather than Sphinx roles.

Files:

  • AGENTS.md
**/*.{py,sh,yaml,yml,md}

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.

Include SPDX copyright headers in all source files, except files explicitly listed in .copyrightignore.

Files:

  • AGENTS.md
**/*

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.

Files:

  • AGENTS.md

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

  • Refactor suggestion: use for local maintainability problems introduced
    by the diff when they have clear future cost, such as duplicated setup,
    unclear boundaries, over-mocking, avoidable complexity, or opaque test
    helpers.
  • Nitpick: avoid in chill mode. Do not emit formatting, import-order,
    wording, or style-only comments unless automated tools cannot catch the
    issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

  • Major: incorrect generation/training/evaluation behavior, broken
    CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
    cleanup and process-isolation bugs likely to fail CI or production
    runs.
  • Minor: localized bugs, missing focused tests for changed behavior, or
    bad test patterns that weaken regression coverage.
  • Trivial: small cleanup with no behavior impact. Usually suppress in
    chill mode.
  • Info: context only. Avoid unless it helps reviewers understand risk.
    Safe-Synthesizer-specific review focus: - Data ...

Files:

  • AGENTS.md
**/*.{py,sh,yaml,yml,toml,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use mise tasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.

Files:

  • AGENTS.md
.github/PULL_REQUEST_TEMPLATE.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Open pull requests using the repository PR template and ensure required review, CI, and conversation-resolution gates are satisfied before merge.

Files:

  • .github/PULL_REQUEST_TEMPLATE.md
.github/**

⚙️ CodeRabbit configuration file

Review GitHub configuration for branch protection expectations, CODEOWNERS alignment, least privilege permissions, pinned actions where practical, and consistency with CONTRIBUTING.md.

Files:

  • .github/PULL_REQUEST_TEMPLATE.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer

Timestamp: 2026-08-12T20:57:57.694Z
Learning: Do not commit unless the user asks for a commit or PR work.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer

Timestamp: 2026-08-12T20:57:57.694Z
Learning: When committing, all commits require DCO sign-off and GPG signing.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer

Timestamp: 2026-08-12T20:57:57.694Z
Learning: Always use `git commit --signoff --gpg-sign` (or `-s -S`) -- never write the `Signed-off-by` trailer manually, and never pass `--no-gpg-sign`.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer

Timestamp: 2026-08-12T20:57:57.694Z
Learning: When creating or updating a pull request, complete the Human review section accurately.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer

Timestamp: 2026-08-12T20:57:57.694Z
Learning: Do not claim that a human reviewed or verified a change unless the human explicitly confirmed it; otherwise select “No human review; automated review only.”
🔇 Additional comments (1)
AGENTS.md (1)

38-39: LGTM!

- [ ] Complete diff reviewed by a human
- [ ] Complete diff reviewed and verified by a human

Verification performed:

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.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Make the verification field explicit for the verified status.

If a contributor selects Complete diff reviewed and verified by a human, the template does not state that Verification performed: is required or what to enter when no verification occurred. This permits a verified status with no supporting record. Add a short condition for this field.

Proposed wording
-Verification performed:
+Verification performed (required for the verified option; write `None` otherwise):
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Verification performed:
Verification performed (required for the verified option; write `None` otherwise):

Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
@binaryaaron
binaryaaron self-requested a review August 13, 2026 21:59
@binaryaaron

Copy link
Copy Markdown
Collaborator

The current PR body selects “Complete diff reviewed and verified by a human,” but the verification record says the agent compared the final diff. Please either record the human’s actual full-diff review and verification or select “No human review; automated review only.” The PR should follow the reporting rule it introduces.

@zywind
zywind requested review from binaryaaron and removed request for binaryaaron August 19, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci area:dev-ex Affects build or dev experience docs Documentation-only change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants