From 9c592118c15c9fd1283e3e6e4b8be80ee0dda7b7 Mon Sep 17 00:00:00 2001 From: "Jason Zhou (Engrg-Hardware 1)" Date: Thu, 17 Sep 2026 18:35:03 -0700 Subject: [PATCH 1/2] docs: clarify risk-tier review handoffs and escalation Signed-off-by: Jason Zhou (Engrg-Hardware 1) --- .github/pull_request_template.md | 8 +++++- CONTRIBUTING.md | 17 ++++++++++++ REVIEW.md | 47 ++++++++++++++++++++++++++++++++ docs/ci.md | 12 ++++++-- 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 11eca93ae..0b91e6086 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -13,9 +13,13 @@ State the problem, the behavior change, and the intended consumer. -- Risk level: +- Risk level and rationale: +- Responsible CODEOWNER: +- Expert escalation: - Start with: - Public or serialized contract changed: - Compatibility or rollback concern: @@ -32,6 +36,8 @@ from CI. - Full CI: - CodeRabbit reviewed commit: - Codex reviewed commit: +- Blocking findings: +- Agreed follow-ups: - Negative or boundary cases: - Expected-value derivation (for exact numerical assertions): - Before/after output, trace, benchmark, or golden diff: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa6b0ffb5..a76a1f423 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,6 +21,23 @@ instructions, see the [application README](python/aisimulate/README.md). - [Office Hours](https://www.youtube.com/playlist?list=PL5B692fm6--tgryKu94h2Zb7jTFM3Go4X) - [Community Meetings](https://docs.google.com/document/d/1uR8xD_hlYGwV6QspvSc36k1H-wo1BUcVmFbHH9xlXd8/view) ([YouTube](https://www.youtube.com/@ai-dynamo-community)) -- Weekly (Wed 10:30 AM PT) development community meetings +## Preparing a pull request + +Use the root [PR template](.github/pull_request_template.md) to describe the +behavior change, validation, and review handoff. Choose low, medium, or high risk +using the [review contract](REVIEW.md#risk-tiered-review-and-ci), name the +responsible CODEOWNER team/reviewer, and identify the expert decision for +high-risk work. Keep unfinished work as a draft: Fast CI still runs, and marking +the PR ready starts eligible automatic CodeRabbit review without a +`review-ready` label. Medium/high risk also requires same-commit Codex review. + +Follow the [handoff and finding policy](REVIEW.md#review-handoff-and-finding-disposition) +when requesting review or responding to findings. Keep blocking conversations +visible, link agreed follow-up issues, and refresh review/CI evidence after each +push. Every tier needs applicable CODEOWNER approval; high risk also needs the +relevant expert. The [CI guide](docs/ci.md#code-review-and-pr-admission) describes +Full CI admission and the separate repository-enforcement requirements. + ## Developer Certificate of Origin AISimulate is an open source project released under the Apache 2.0 license diff --git a/REVIEW.md b/REVIEW.md index 9b2002ae5..6f09ad799 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -39,6 +39,53 @@ approval. | Medium | Fast CI, CodeRabbit, and Codex | Applicable CODEOWNER | | High | Fast CI, CodeRabbit, and Codex | CODEOWNER plus relevant domain, architecture, security, or release owner | +Choose the highest tier warranted by the changed behavior and explain it in the +PR's review map. Low risk covers documentation and mechanical changes with no +runtime or enforcement change. Medium risk covers bounded behavior changes with +known consumers and reproducible validation. High risk includes numerical-model +or performance-data changes, public compatibility, cross-layer architecture, +and changes to security, runner trust, or release authority. A small diff is not +by itself low risk. The responsible CODEOWNER confirms the tier; a bot suggestion +can prompt escalation but cannot waive the required reviews. + +### Review handoff and finding disposition + +Before marking a PR ready for review, fill in the template's risk rationale, +responsible CODEOWNER, and expert escalation fields. Use the generated root +[CODEOWNERS](CODEOWNERS) and GitHub's requested-reviewer list to route the changed +paths. Name the owning team while an individual reviewer is being assigned; +record the responsible reviewer once that person accepts the handoff. For +changes spanning owners, identify who coordinates the handoff and request the +other affected owners. A GitHub code-owner approval can be satisfied by one of +several listed owners; it does not prove every relevant expert reviewed the +change. + +For high-risk work, name the domain, architecture, security, or release owner +and the question requiring their decision. The same person may cover CODEOWNER +and expert roles when qualified; record both roles explicitly. For other tiers, +write N/A or name the expert needed for a specific uncertainty. Route an +unanswered request or a disputed tier/finding through the owning team, then the +repository maintainers if needed. Routine handoffs do not require a particular +maintainer or project lead. + +Reviewers should acknowledge an accepted handoff, give an expected review time +or name a replacement, and distinguish **blocking** findings from **follow-up** +suggestions. Authors should answer each actionable thread with the fixing +commit and validation, a reasoned disagreement, or an agreed follow-up issue. +If review or a fix is delayed, update the PR with the owner and next step; do not +treat silence as approval. + +Keep blocking findings in GitHub review conversations until corrected or the +reviewer accepts an evidence-backed disposition. P0/P1 findings block Full CI +admission; other required findings may be addressed while Full CI runs but still +block merge. A follow-up is non-blocking only when the responsible reviewer +agrees and the PR links an issue with an owner and scope. Do not hide a blocker +by resolving its thread or moving it to the backlog. After a push, refresh the +reviewed SHA, relevant reviews, CI links, and outstanding findings in the PR; +review completion requires the current head and resolved required conversations. + +### CI evidence and admission + Fast CI contains quick deterministic checks: source and legal policy, generated CODEOWNERS integrity, lint, syntax compilation, whitespace, and Rust formatting. Full CI contains the expensive multi-architecture dependency, Rust, Python, diff --git a/docs/ci.md b/docs/ci.md index 1907dd2cf..0b7af51ba 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -164,15 +164,21 @@ configures automated review separately from GitHub Actions. 1. Open or update the PR to run Fast CI automatically, including while it is a draft. Mark the PR non-draft for automatic CodeRabbit review, subject to its configured title and label exclusions. No `review-ready` label is needed. -2. Complete the reviews required for the risk level on the current commit: - CodeRabbit for all tiers, plus Codex for medium/high risk. +2. Record the risk tier and rationale, responsible CODEOWNER team/reviewer, and + expert escalation in the PR's review map, following the + [handoff policy](../REVIEW.md#review-handoff-and-finding-disposition). + Complete CodeRabbit review for all tiers, plus same-commit Codex review for + medium/high risk. Link reviewed SHAs and validation evidence in the PR; a + skipped bot review or green status alone is not completed review. 3. A maintainer admits Full CI after the required initial reviews complete with no unresolved P0/P1 finding. Lower-priority findings and CODEOWNER review may proceed while Full CI runs. 4. Before merge, confirm the current head, applicable CODEOWNER approval, required conversation resolution, CI results, and effective branch rules. High-risk changes also need the relevant domain, architecture, security, - or release owner. + or release owner. Keep blockers in GitHub conversations and move only agreed + non-blocking follow-ups to linked issues with an owner. Refresh this evidence + after every push; a review or CI result for an older head is historical. Trusted copy-pr-bot branches also launch Full CI automatically as a coverage backstop. Check that the copy's SHA matches the PR head before using its result. From ce7b315e920b84f5c0bd17435b2f3849898e8844 Mon Sep 17 00:00:00 2001 From: "Jason Zhou (Engrg-Hardware 1)" Date: Thu, 17 Sep 2026 18:43:25 -0700 Subject: [PATCH 2/2] docs: clarify admission for excluded CodeRabbit reviews Signed-off-by: Jason Zhou (Engrg-Hardware 1) --- .github/pull_request_template.md | 3 ++- CONTRIBUTING.md | 4 +++- REVIEW.md | 7 +++++++ docs/ci.md | 4 ++++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 0b91e6086..e72aeb9f2 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -14,7 +14,8 @@ State the problem, the behavior change, and the intended consumer. - Risk level and rationale: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a76a1f423..9e71c88ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,9 @@ using the [review contract](REVIEW.md#risk-tiered-review-and-ci), name the responsible CODEOWNER team/reviewer, and identify the expert decision for high-risk work. Keep unfinished work as a draft: Fast CI still runs, and marking the PR ready starts eligible automatic CodeRabbit review without a -`review-ready` label. Medium/high risk also requires same-commit Codex review. +`review-ready` label. Remove title/label exclusions when requesting review; +a skipped CodeRabbit check does not qualify the PR for maintainer Full CI +admission. Medium/high risk also requires same-commit Codex review. Follow the [handoff and finding policy](REVIEW.md#review-handoff-and-finding-disposition) when requesting review or responding to findings. Keep blocking conversations diff --git a/REVIEW.md b/REVIEW.md index 6f09ad799..7f395850b 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -33,6 +33,13 @@ also require a Codex review of the same commit. The risk level changes review depth, not merge authority: every tier still requires the applicable CODEOWNER approval. +A draft or a PR excluded by the configured title/label filters is not eligible +for maintainer Full CI admission on the strength of a skipped CodeRabbit check. +When the work is ready, remove those exclusions, mark it non-draft, and obtain +substantive CodeRabbit review of the current head. There is no replacement-review +waiver in this policy. An automatic copy-branch coverage run does not satisfy +this review requirement or grant merge permission. + | Risk | Review before Full CI | Human merge gate | | --- | --- | --- | | Low | Fast CI and CodeRabbit | Applicable CODEOWNER | diff --git a/docs/ci.md b/docs/ci.md index 0b7af51ba..127d828ea 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -164,6 +164,10 @@ configures automated review separately from GitHub Actions. 1. Open or update the PR to run Fast CI automatically, including while it is a draft. Mark the PR non-draft for automatic CodeRabbit review, subject to its configured title and label exclusions. No `review-ready` label is needed. + If review is skipped because the PR is a draft or excluded, clear those + conditions and obtain current-head CodeRabbit review before maintainer Full + CI admission. The automatic copy-branch coverage backstop does not waive + this requirement; no replacement-review exception is defined. 2. Record the risk tier and rationale, responsible CODEOWNER team/reviewer, and expert escalation in the PR's review map, following the [handoff policy](../REVIEW.md#review-handoff-and-finding-disposition).