Skip to content

docs(codereview): require design context for deep PRs - #550

Merged
enyst merged 3 commits into
mainfrom
docs/deep-pr-design-context
Oct 4, 2026
Merged

enyst merged 3 commits into
mainfrom
docs/deep-pr-design-context

Conversation

@enyst

@enyst enyst commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

HUMAN:
This PR proposes to request a design doc for deep enough changes; e.g. API changes or shared execution/loading or other contracts. The custom review guide request is similar with others in other repos.


AGENT:

Why

The extensions repository has the .pr/ artifact workflow but no repository-specific guidance on when a deep, high-risk change needs design context. Reviewers (human and automated) end up reconstructing intent from the diff, and an automated approval can delete the only .pr/ design page before a human sees it. This adds that expectation to the repository review guide, scoped to extensions-owned contracts, without assuming a repository-local pr-design-doc skill.

Summary

  • .agents/skills/custom-codereview-guide.md: a new Design context for deep changes checkpoint under "Blocking checkpoints".
  • Names the signals for deep changes (skill/plugin/automation/integration contracts, manifest schemas, migrations, shared loading/validation/discovery/execution, large non-mechanical diffs) and the changes that need nothing (typos, one-line guards, dependency bumps, small docs edits, localized fixes). Line count is a signal, never a gate.
  • Defines acceptable context (a design-doc artifact or a durable PR-description write-up with intent, before/after behavior or contract shape, compatibility and risk, and code references) and weighs a missing one by the HIGH/MEDIUM/LOW risk levels the code-review skill already uses.
  • Prevents an automated approval from deleting a .pr/ page that is the only design explanation.
  • The branch is merged with current main; the section was rewritten to fit the guide's restructuring in docs(review): define extension-specific checkpoints #611 and docs(review): define extensions repository scope #643.

Issue Number

Fixes #549

How to Test

Ran in the PR worktree:

git diff --check
grep -q '^triggers:' .agents/skills/custom-codereview-guide.md

Both pass. Checked each acceptance criterion of #549 against the new section:

  • HIGH-risk deep PRs without design context get a COMMENT review, not approval.
  • Low-risk, trivial, generated, or self-explanatory changes are not blocked by design-doc absence or line count.
  • Equivalent context must state intent, before/after behavior or contract shape, compatibility/risk, and grounded code references.
  • The guide does not claim .agents/skills/pr-design-doc/ exists here.
  • A .pr/ page that is the only design context blocks automated approval (approval triggers cleanup-on-approval in .github/workflows/pr-artifacts.yml).

This is review-guide text; it takes effect on the next reviewer run that loads the guide.

Video/Screenshots

Not applicable; no product UI change.

Notes

The general code-review skill already withholds approval for HIGH risk; this checkpoint makes missing design context an explicit input to that decision for this repository.

This pull request was created by an AI agent (OpenHands) on behalf of @enyst, and updated by an AI agent (Claude Code, Opus 5.5) helping Engel Nyst (@enyst).

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions github-actions Bot added the type: docs Documentation only changes label Sep 11, 2026
enyst and others added 2 commits October 4, 2026 21:33
…uide

Main reorganized the review guide into blocking checkpoints since this
branch was opened. Move the deep-PR design-context rules into that
structure as one checkpoint, keep every acceptance criterion from #549,
and use the risk levels the code-review skill already defines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@enyst
enyst requested a review from all-hands-bot October 4, 2026 19:34

@all-hands-bot all-hands-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.

This review was posted by an AI agent (OpenHands).

Scope: In scope for OpenHands/extensions. The change adds repository-local review guidance to .agents/skills/custom-codereview-guide.md, which this repository owns. Linked issue #549 is open and carries enhancement + ready-for-dev, so the work has a product/architecture direction and does not need to move repositories.

What I verified on head 701a545:

  • The diff is exactly +31/-0 in one file; no other files changed.
  • The .pr/ cleanup claim is accurate: .github/workflows/pr-artifacts.yml runs cleanup-on-approval on pull_request_review with state == 'approved' and head.repo.full_name == github.repository, and it git rm -rf .pr/. The new text's caution that a .pr/-only design page must not be auto-approved therefore matches the workflow's real behavior.
  • The HIGH/MEDIUM/LOW weighting matches skills/code-review/SKILL.md and references/risk-evaluation.md ("a HIGH risk assessment requires a COMMENT ... do not approve it for automatic merge; LOW or MEDIUM risk alone does not justify withholding approval"). No contradiction with the general skill.
  • The guide does not claim .agents/skills/pr-design-doc/ exists; the directory contains only custom-codereview-guide.md.
  • Repo punctuation convention (plain hyphens, no em dashes) is respected in the added lines.
  • git diff --check passes and grep -q '^triggers:' passes, as the PR states. No test asserts on this guide's content, so the prose change carries no regression risk.
  • Current-head checks for 701a545 are green: pr-title, sync-extensions, validate-claude-code, sync-sdk-skill, test, check, check-pr-artifacts, package. cleanup-on-approval is skipped (expected until an approval event fires).

Findings: None material. The guidance is specific and operational, is scoped to the extensions-owned contracts the issue names, and its acceptance criteria are each addressed by the added section.

Non-blocking note: The PR is still marked draft and the HUMAN: section of the template is unedited. That is a workflow/handoff concern owned by the deterministic event handler, not a defect in the change itself, so it does not affect this verdict.

Verdict: The change is a documentation-only addition to the repository review guide, is accurate against the repository's own machinery, and introduces no correctness, security, or compatibility risk. No material findings.

✅ APPROVED

@all-hands-bot
all-hands-bot requested a review from neubig October 4, 2026 19:41
@enyst
enyst marked this pull request as ready for review October 4, 2026 21:02
@enyst
enyst merged commit a46bd3c into main Oct 4, 2026
22 checks passed
@enyst
enyst deleted the docs/deep-pr-design-context branch October 4, 2026 21:03
@openhands-release-bot openhands-release-bot Bot added the released: v0.28.0 Shipped in v0.28.0 label Oct 5, 2026
@openhands-release-bot

Copy link
Copy Markdown
Contributor

🚀 Released in v0.28.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released: v0.28.0 Shipped in v0.28.0 type: docs Documentation only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define design-doc expectations for deep, high-risk PRs

2 participants