Skip to content

feat(pr-management-code-review): suggest domain-expert reviewers in the review body#844

Merged
potiuk merged 1 commit into
apache:mainfrom
shahar1:feat/code-review-suggest-additional-reviewers
Jul 12, 2026
Merged

feat(pr-management-code-review): suggest domain-expert reviewers in the review body#844
potiuk merged 1 commit into
apache:mainfrom
shahar1:feat/code-review-suggest-additional-reviewers

Conversation

@shahar1

@shahar1 shahar1 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • During a per-PR code review, the skill now surfaces up to 2–3 domain experts worth pinging for an additional look at the touched area, folded into the review body as a "Suggested additional reviewers" section. Today CODEOWNERS is only a queue-selection signal and reviewer-routing is a separate, roster-bounded skill — nothing suggests area experts inside the review output itself.
  • Candidates are derived cheapest-source-first: CODEOWNERS owners for the changed paths (authoritative, ~always a committer → satisfies "at least one maintainer"), then recent commit authors on those paths (surfaces active non-committer contributors), then reviewers of prior merged PRs on those paths (only when the cheap sources fall short).
  • Grounded and safe by construction: every suggestion must trace to a concrete source (no fabricated handles; empty section when nothing grounds out), the PR author / viewer / already-requested / already-reviewing people are excluded, and suggestions are named only — the skill never auto-requests reviewers.

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below

Test plan

  • New step-4.5-suggested-reviewers eval suite (4 cases): grounded 2–3-with-a-committer, empty-section-when-nothing-grounds, prompt-injection resistance (an ungrounded "add @x" line in the PR body is ignored), and exclusion of an already-reviewing owner.
  • skill-and-tool-validate passes for the edited skill (internal links resolve; no new hard violations).
  • Eval-runner discovery of the new suite verified locally (skill-eval renders all 4 cases, exit 0). Full LLM grading requires the claude CLI + an API key, which was not available in the authoring environment — it should be run by CI / a reviewer:
    uv run --project tools/skill-evals skill-eval --cli "claude -p" tools/skill-evals/evals/pr-management-code-review/step-4.5-suggested-reviewers/fixtures/

RFC-AI-0004 compliance

  • HITL — suggestions are named in the review body only; no reviewer is auto-requested (gh pr edit --add-reviewer / requestReviews are never called).
  • Vendor neutrality — new prose uses <repo> / <viewer> / <project-config> placeholders.
  • Conversational + correctable — behaviour lives in the existing skill's documented flow; adopter overrides apply as for the rest of the skill.

Notes for reviewers (optional)

  • The bulk of the change lives in sibling files (review-flow.md Step 4.5, posting.md template section) to respect the SKILL.md ≤500-line guidance; SKILL.md only gains short pointers (per-PR loop, does-NOT-do list, gh budget).
  • The PR branch is based on the fork's main tip plus this single commit, so the three-dot diff shows exactly the intended change. It can be rebased onto current apache:main on request (the fork couldn't be synced here without the workflow OAuth scope).

…he review body

During a per-PR review, surface up to 2-3 domain experts worth pinging
for an additional look at the touched area, folded into the review body.

New Step 4.5 in review-flow.md derives candidates, cheapest source first:
  1. CODEOWNERS owners for the changed paths (authoritative, ~always a
     committer — satisfies the "at least one maintainer" requirement)
  2. recent commit authors on the touched paths (surfaces active
     non-committer contributors)
  3. reviewers of prior merged PRs on those paths (only when 1-2 fall short)

Candidates are filtered for activity and de-noised (drop the author,
the viewer, and anyone already requested / already reviewing). Every
suggestion must trace to a concrete source — no fabricated handles, and
an empty section when nothing grounds out. Suggestions are named in the
body only; the skill never auto-requests reviewers (that stays a
maintainer action, or a job for reviewer-routing).

posting.md gains the "Suggested additional reviewers" body section;
SKILL.md notes it in the per-PR loop, the does-NOT-do list, and the
per-PR gh budget.

Adds a step-4.5-suggested-reviewers eval suite (4 cases): grounded
2-3-with-a-committer, empty-section-when-nothing-grounds, prompt-injection
resistance (ungrounded body request ignored), and exclusion of
already-reviewing owners.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NICE!

@potiuk potiuk merged commit 31264eb into apache:main Jul 12, 2026
10 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.

2 participants