Conversation
Add a "Design Docs for Deep PRs" section to the custom code-review guide. It describes the changes a design doc is expected for (new/changed automation contract or webhook/dispatch API, new module/subsystem, cross-cutting refactor or migration, core-behavior changes) and skips trivial PRs. Instructs the reviewer to withhold approval for a deep HIGH-risk PR with no design doc, and to use judgment (prefer withholding) for a deep MEDIUM-risk PR without one. LOW risk never blocks. Ties into the pr-design-doc skill. Co-authored-by: smolpaws <engel@enyst.org> Co-authored-by: openhands <openhands@all-hands.dev>
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
A large diff whose intent a reviewer cannot hold in their head at once is also a case where a design doc helps, even if no single hunk is complex. Co-authored-by: smolpaws <engel@enyst.org> Co-authored-by: openhands <openhands@all-hands.dev>
enyst
left a comment
enyst
left a comment
There was a problem hiding this comment.
🔴 Needs improvement
[CRITICAL ISSUES]
- The automated approval policy conflicts with same-repository
.pr/cleanup and can remove the only design artifact before human review. See the inline finding.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
The patch is documentation-only, but it changes whether the designated automated reviewer supplies the repository's required approval for deep automation changes.
VERDICT:
❌ Needs rework: Require durable design context or prior human review before automated approval.
KEY INSIGHT:
The designated approval bot must not destroy the evidence a later human reviewer needs.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add or edit
.agents/skills/custom-codereview-guide.mdwith the/codereviewtrigger and the missing repository context. See the customization docs.- Re-request a review; guidelines from the PR branch take effect immediately.
- When merged, the guideline changes receive normal repository review.
Resolve with AI? Install the iterate skill and run
/iterate.Was this review helpful? React with 👍 or 👎.
This review was generated by an AI agent (OpenHands) on behalf of @enyst.
| When a deep PR ships without a design doc, weigh the omission against the | ||
| change's risk assessment: | ||
|
|
||
| - **🔴 HIGH risk and deep, no design doc:** withhold approval. Submit the |
There was a problem hiding this comment.
🔴 Critical — approval removes the evidence too early: The repository's PR Artifacts workflow removes .pr/ immediately after any same-repository approval. Because this bot's approval satisfies the required review gate, approving on the strength of a branch-linked design doc can delete that page before a human maintainer reads it. Require either prior human-maintainer review while the doc is present or durable equivalent context in the PR description with a minimum rubric: intent, important before/after behavior or API shape, compatibility/risk, and grounded code references.
3 participants
Why
The automation repository already provides the
pr-design-docskill, but its automated review guide does not say when reviewers should expect design context. This follow-up makes that expectation explicit for deep, high-risk scheduling, webhook, event, and dispatch changes while exempting small or self-explanatory diffs.Summary
Issue Number
Fixes #443
How to Test
From the PR head, the following lightweight validation passed:
git diff --check origin/main...pr-441 grep -q '^triggers:' .agents/skills/custom-codereview-guide.mdThe PR's required description and title checks pass. Review the new decision matrix against
.agents/skills/pr-design-doc/SKILL.mdand the repository's required approval-review policy.Video/Screenshots
Not applicable. This changes repository-local Markdown review guidance and has no product UI.
Type
Notes
Review identified one lifecycle concern still requiring author action: same-repository
.pr/artifacts are removed as soon as an automated approval is submitted, which can make a branch-linked design doc disappear before a human maintainer reads it. The policy should require durable equivalent context in the PR description before approval, or otherwise reconcile cleanup timing.This PR description was updated by an AI agent (OpenHands) on behalf of @enyst.